Foundation Drop contest - 0xHarry's results

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L210-L216 https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L125-L136

Vulnerability details

Impact

In MarketFees _distributeFunds iterates over an array of creatorRecipients and send each creator his share in ether. Every call is given a gas limit to not inflate the gas used. In this for loop however, each creator gets the gas limit of multiple Recipients.

   // Pay the creator(s)
   unchecked {
     // @audit could be an unbound for loop and run out of gas
     for (uint256 i = 0; i < creatorRecipients.length; ++i) {
       _sendValueWithFallbackWithdraw(
         creatorRecipients[i],
         creatorShares[i],
         // @audit is only one recipient wrong gas limit used
         SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS
       );
       // Sum the total creator rev from shares
       // creatorShares is in ETH so creatorRev will not overflow here.
       creatorRev += creatorShares[i];
     }
   }

This could easily inflate the gas used and if the array is big enough can fill up the block gas limit and therefore fail the call

Proof of Concept

mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L210-L216 mixins/shared/MarketFees.sol#L125-L136

Tools Used

manual analysis.

Recommended Mitigation Steps

change SEND_VALUE_GAS_LIMIT_MULTIPLE_RECIPIENTS to SEND_VALUE_GAS_LIMIT_SINGLE_RECIPIENT

Storage variables should be loaded into memory if they are read multiple times and before applying side effects.

This can be done with: versionNFTCollection in https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L202-L218 and versionNFTDropCollection in https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L226-L247