Lines of code

Vulnerability details

According to the README.md

> If royalty information was not defined when the NFT was originally deployed, it may be added using the Royalty Registry which will be respected by our market contract.

<https://github.com/code-423n4/2022-02-foundation/blob/4d8c8931baffae31c7506872bf1100e1598f2754/README.md?plain=1#L21&gt;

The actual exchange code only respects the Royalty Registry or other royalty information if the number of recipients is less than or equal to four.

Impact

If the creatorRecipients.length is more than four then the array is essentially truncated and the royalties are only split among the first four entries in the array. If the array happens to be sorted from low to high then the people who were supposed to get the largest portions of the royalties are given nothing.

Proof of Concept

        uint256 maxCreatorIndex = creatorRecipients.length - 1;
        if (maxCreatorIndex > MAX_ROYALTY_RECIPIENTS_INDEX) {
          maxCreatorIndex = MAX_ROYALTY_RECIPIENTS_INDEX;
        }

<https://github.com/code-423n4/2022-02-foundation/blob/4d8c8931baffae31c7506872bf1100e1598f2754/contracts/mixins/NFTMarketFees.sol#L76-L79&gt;

        // Send payouts to each additional recipient if more than 1 was defined
        uint256 totalDistributed;
        for (uint256 i = 1; i <= maxCreatorIndex; ++i) {
          uint256 share = (creatorFee * creatorShares[i]) / totalShares;
          totalDistributed += share;
          _sendValueWithFallbackWithdraw(creatorRecipients[i], share, SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS);
        }

<https://github.com/code-423n4/2022-02-foundation/blob/4d8c8931baffae31c7506872bf1100e1598f2754/contracts/mixins/NFTMarketFees.sol#L99-L105&gt;

Creators shouldn’t have to settle the correct amounts amongst themselves afterwards and doing so may trigger unwanted tax consequences for the creators who got the larger shares of funds.

Tools Used

Code inspection

Recommended Mitigation Steps

Fetch the royalty information during offer creation, cache it for the final transfer, and reject any NFT for which the array size is more than MAX_ROYALTY_RECIPIENTS_INDEX

---

The text was updated successfully, but these errors were encountered:

All reactions