code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

Input [limitperAccount] in NFTDropMarket.createFixedPriceSale() is easily avoidable in one transaction #263

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

It is designed to cap one buys per account by limitperAccount configured during when sale is created. It should revert with NFTDropMarketFixedPriceSale_Cannot_Buy_More_Than_Limit() if (IERC721(nftContract).balanceOf(msg.sender) + count > saleConfig.limitPerAccount) Thus to pass this check IERC721(nftContract).balanceOf(msg.sender) should be minimized. It is very easy to do in one transaction, if buyer is a smart-contract (buy->transfer->buy->transfer) As the result, anyone can buy as much NFT as he wish, even all the collection. But the seller expect that the limit works.

Proof of Concept

Steps - in one transaction from a smart-contract: 1) Buy one 2) Transfer this one somewhere 3) Buy new one 4) Transfer again ...and so one N) Transfer all to the one EOA Result - as many tokens per one account as desired.

Tools Used

Hardhat

Recommended Mitigation Steps

Options: 1) delete this input, accept that it not possible to limit buys per user 2) only EOA should buy 3) block sales if tx.origin does not change 4) block transfers when sale is on 5) something more smart, it is many option possible

HardlyDifficult commented 1 year ago

Dupe of https://github.com/code-423n4/2022-08-foundation-findings/issues/59