sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

avoloder - If there are more than 255 attributions, only first 255 will get the fee distribution #386

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

avoloder

high

If there are more than 255 attributions, only first 255 will get the fee distribution

Summary

Since there is no hard limit on how many attributions one collection can have, it means that it could potentially have multiple hundreds of them. However, only the first 255 attributions will receive their fee distribution, while the rest won't. Depending on how significant the fees are, missing them could result in an indirect loss for other users.

Vulnerability Detail

When publishing the new work in the given edition, one can add multiple attributions for individuals who are supposed to receive fees for their models being used. This functionality can be observed in the _publishfunction of the TitlesCore.sol file.

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/TitlesCore.sol#L120-L149

In the same function the function createRoute of the FeeManager.sol contract is called and the attributions are forwarded. Furthermore, createRoute function calls the _buildSharesAndTargets further passing the attributions array.

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/fees/FeeManager.sol#L125-L160

However, if we take a closer look at the _buildSharesAndTargets function, attribution shares are casted to uint32 attributions array length uint32 attributionShares = uint32(attributions.length);.

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/fees/FeeManager.sol#L476-L498

Later on, we go over attributionShares but defining the counter of the for loop as uint8 which means that it can only count until 255, preventing other attributions to get their fees.

for (uint8 i = 0; i < attributionShares; i++) {
            targets[i + 1] = attributions[i].target;
            shares[i + 1] = attributionRevShare;
        }

Although the sponsor mentioned in a Discord a limit of 255 attributions, this limit was not explicitly stated in either the README or the contract logic, which means that it is feasible with the current state of the contracts. Also in the SplitV2.sol contract there is no limit on how many recipients are allowed.

Impact

Many users may not receive their fees

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/d7f60952df22da00b772db5d3a8272a988546089/wallflower-contract-v2/src/fees/FeeManager.sol#L476-L498

Tool used

Manual Review

Recommendation

Set the hard limit in the contract and either use it as a modifier or include a require statement to check the maximum length of the attribute array.

Duplicate of #277