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

0 stars 0 forks source link

Minter or Creator can NFT for free as the cost of the user. #236

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

In the following code in NFTDropCollection,

   */
  function mintCountTo(uint16 count, address to) external onlyMinterOrAdmin returns (uint256 firstTokenId) {
    require(count != 0, "NFTDropCollection: `count` must be greater than 0");

    unchecked {
      // If +1 overflows then +count would also overflow, unless count==0 in which case the loop would exceed gas limits
      firstTokenId = latestTokenId + 1;
    }
    latestTokenId = latestTokenId + count;
    require(latestTokenId <= maxTokenId, "NFTDropCollection: Exceeds max tokenId");

    for (uint256 i = firstTokenId; i <= latestTokenId; ) {
      _mint(to, i);
      unchecked {
        ++i;
      }
    }
  }

minter or admin can mint NFT for free, which is not fair to the user.

Proof of Concept

Provide direct links to all referenced code on GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

the mintCountTo Function is not payable, so minter or admin can mint NFT for free at the cost of the user.

Tools Used

Recommended Mitigation Steps

We can modifer the function signature.

  function mintCountTo(uint16 count, address to, uint256 moneySpentInMint) payable external onlyMinterOrAdmin returns (uint256 firstTokenId) {
   require(moneySpentInMint == msg.value, 'invalid mint amount')
   // other logic
}   

to make sure no one can mint for free.

HardlyDifficult commented 2 years ago

Invalid -- it's a design choice to allow creator's to mint from their own collection.

Since this is limited to the creator already, requiring a fee would not really make a difference since the money goes right back to their wallet anyways.

Creator's may want to mint a few for themselves, as a long term play. They may also choose to airdrop some NFTs to important collectors. We wanted to remain flexible for use cases such as those.

HickupHH3 commented 2 years ago

feature, not bug