Foundation Drop contest - auditor0517's results

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L183

Vulnerability details

Impact

Originally, saleConfig.limitPerAccount is used to prevent a user can't mint more than limitPerAccount.

But users can bypass this requirement by transferring their already minted nfts to other addresses before mint.

Proof of Concept

NFTDropCollection has default transfer() and transferFrom() functions of ERC721Upgradeable so that users can transfer their nfts as they want.

So if a user wants to buy more nfts than saleConfig.limitPerAccount, he can transfer already minted nfts to another address so that his balance is 0 here.

So he can purchase up to saleConfig.limitPerAccount of nfts again.

Tools Used

Manual Review

Recommended Mitigation Steps

After a discussion with the sponsor, I understood there is no method to improve this logic.

Currently, saleConfig.limitPerAccount is only used as the maximum number of nfts to purchase a time.

Maybe we should consider removing this limitation logic for a clearer structure.

Low Risk Issues

[L-01] Wrong comment

versionNFTCollection and versionNFTDropCollection are uint32 variables.

• https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L206

File: 2022-08-foundation\contracts\NFTCollectionFactory.sol
206:       // Version cannot overflow 256 bits.

• https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollectionFactory.sol#L230

File: 2022-08-foundation\contracts\NFTCollectionFactory.sol
230:       // Version cannot overflow 256 bits.

[L-02] Unbounded loops with external calls

• https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L181-L186

File: 2022-08-foundation\contracts\NFTDropCollection.sol
181:     for (uint256 i = firstTokenId; i <= latestTokenId; ) {
182:       _mint(to, i);
183:       unchecked {
184:         ++i;
185:       }
186:     }

Non-critical Issues

[N-01] Open TODO

• https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L193

File: 2022-08-foundation\contracts\mixins\shared\MarketFees.sol
188:     (totalFees, creatorRecipients, creatorShares, sellerRev, ) = _getFees(
189:       nftContract,
190:       tokenId,
191:       seller,
192:       price,
193:       // TODO add referral info
194:       payable(0)
195:     );

[N-02] Each event should use three indexed fields if there are three or more fields

• https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol#L79-L84

File: 2022-08-foundation\contracts\mixins\nftDropMarket\NFTDropMarketFixedPriceSale.sol
79:   event CreateFixedPriceSale(
80:     address indexed nftContract,
81:     address indexed seller,
82:     uint256 price,
83:     uint256 limitPerAccount
84:   );

• https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/mixins/shared/MarketFees.sol#L71-L77

File: 2022-08-foundation\contracts\mixins\shared\MarketFees.sol
71:   event BuyReferralPaid(
72:     address indexed nftContract,
73:     uint256 indexed tokenId,
74:     address buyReferrer,
75:     uint256 buyReferrerFee,
76:     uint256 buyReferrerSellerFee
77:   );