QA Report

[code423n4]

Missing token whitelisting puts stakeholders on risk

Contract: https://github.com/code-423n4/2022-02-nested/blob/main/contracts/FeeSplitter.sol#L146

1. Attacker can call sendFees with a malicious token contract

2. This increases the share balance of malicious token for each stake holder

3. When stakeholders tries to withdraw there share of malicious token using releaseTokens, malicious contract will be called and code written by attacker will be executed (asking for unauthorized approvals, wasting Gas etc)

sendFees & sendFeesWithRoyalties not handling ETH token

Contract: https://github.com/code-423n4/2022-02-nested/blob/main/contracts/FeeSplitter.sol#L175

1. Both sendFees & sendFeesWithRoyalties are not considering if the input _token is ETH as done in releaseTokens

Incorrect return message

Contract: https://github.com/code-423n4/2022-02-nested/blob/main/contracts/abstracts/MixinOperatorResolver.sol#L101

1. The require statement incorrectly mentions INVALID_OUTPUT_TOKEN when it should be INVALID_INPUT_TOKEN

[maximebrugel]

"Missing token whitelisting puts stakeholders on risk" (Disputed)

Yes and no... Yes it can run malicious code in the transfer function, but we can skip the token if we want.
Same logic can be applied to NestedFactory. We can't really change that since we are not "whitelisting" tokens.

"sendFees & sendFeesWithRoyalties not handling ETH token" (Disputed)

We are only sending fees with ERC20 (so WETH and not ETH). In the releaseTokens tokens we are unwrapping the WETH to transfer ETH.

"Incorrect return message" (Confirmed)

[maximebrugel]

Fixed in #70 commit

[harleythedogC4]

My personal judgements:

1. "Missing token whitelisting puts stakeholders on risk". User's don't need to interact with tokens if they don't want to. Invalid
2. "sendFees & sendFeesWithRoyalties not handling ETH token". As sponsor describes. Invalid.
3. "Incorrect return message". Valid and non-critical.

[harleythedogC4]

Also adding in the reduced severity finding #10:
4. "Royalty owner can steal Stakeholders fees". Just code consistency. Valid and non-critical.

[harleythedogC4]

Now, here is the methodology I used for calculating a score for each QA report. I first assigned each submission to be either non-critical (1 point), very-low-critical (5 points) or low-critical (10 points), depending on how severe/useful the issue is. The score of a QA report is the sum of these points, divided by the maximum number of points achieved by a QA report. This maximum number was 26 points, achieved by #66.

The number of points achieved by this report is 2 points.
Thus the final score of this QA report is (2/26)*100 = 8.