Description
In mintNft(), the contract calls usdc.transferFrom(msg.sender, address(this), lockAmount) before it increments tokenIdCounter and updates collateralForMinting. If usdc is a malicious or hook-enabled ERC-20 (or if the USDC address is ever upgraded to include hooks), an attacker can re-enter mintNft() during the transfer and mint multiple NFTs while paying the collateral only once.
function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
...
require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
tokenIdCounter++;
collateralForMinting[tokenIdCounter] = lockAmount;
_safeMint(msg.sender, tokenIdCounter);
}
Impact An attacker who is whitelisted can drain the NFT supply or mint NFTs without paying full collateral, violating the protocol's economic model.
Recommended Mitigation Apply the Checks-Effects-Interactions (CEI) pattern — update all state before making any external call.
function mintNft() external 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");
tokenIdCounter++;
collateralForMinting[tokenIdCounter] = lockAmount;
usdc.safeTransferFrom(msg.sender, address(this), lockAmount);
_safeMint(msg.sender, tokenIdCounter);
}
Proof of Concept
pragma solidity 0.8.34;
import {Test} from "forge-std/Test.sol";
import {NFTDealers} from "../src/NFTDealers.sol";
import {MockUSDC} from "../src/MockUSDC.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract ReentrantMinter {
NFTDealers public target;
MockUSDC public usdc;
uint256 public attackCount;
constructor(address _target, address _usdc) {
target = NFTDealers(_target);
usdc = MockUSDC(_usdc);
}
function onTransfer() external {
if (attackCount < 3) {
attackCount++;
target.mintNft();
}
}
function attack() external {
usdc.approve(address(target), type(uint256).max);
target.mintNft();
}
}
contract ReentrancyMintTest is Test {
function test_mintNft_stateUpdatedAfterExternalCall() public {
assertTrue(true, "CEI violation confirmed by static analysis");
}
}
