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

0 stars 0 forks source link

users could not mint if they achieve the `limitPerAccount` by shopping in the secondary market #220

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

The users could lose their rights to mint all the limitPerAccount or some of them

Proof of Concept

If any user buys these NFTs from the secondary market or just he minted with another address and then transfers it to this address. In these cases, this user can’t bypass this check

File:        /main/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol

if (IERC721(nftContract).balanceOf(msg.sender) + count > saleConfig.limitPerAccount) {
      if (saleConfig.limitPerAccount == 0) {
        // Provide a more targeted error if the collection has not been listed.
        revert NFTDropMarketFixedPriceSale_Must_Have_Sale_In_Progress();
      }
      revert NFTDropMarketFixedPriceSale_Cannot_Buy_More_Than_Limit(saleConfig.limitPerAccount);
    }

even though he never mint in this collections

Recommended Mitigation Steps

Add check to inquire about their NFTs come from or check if he achieves the limitPerAccount by minting not just check the balanceOf(msg.sender)

ghost commented 2 years ago

This issue overlaps with the issue of "bypass account limits" but I'm not entirely sure if the warden is trying to highlight that based on how this issue is worded.

HardlyDifficult commented 2 years ago

It's a different scenario perspective on the same root cause. I feel it's a dupe but not opposed to tracking these separately.

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

HickupHH3 commented 2 years ago

Agreed. Different perspective on the same issue. Keeping it marked as a dup.