NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

NFTDealers Audit Report

Author Revealed upon completion

External call to usdc.transferFrom (and _safeMint) happens before critical state updates, with no reentrancy guard.

Description

  • Describe the normal behavior in one or more sentences

    When a whitelisted user calls mintNft(), the contract takes the required USDC collateral, increments the token counter, stores the collateral amount for that token, and safely mints a new NFT to the caller. If the collection isn’t revealed, the user isn’t whitelisted, the caller is the owner, or max supply is reached, it reverts.

  • Explain the specific issue or problem in one or more sentences

    mintNft() makes an external call to usdc.transferFrom before it updates critical state, and then calls _safeMint, which can invoke user code via onERC721Received. A malicious token or receiver can reenter mintNft() during those external calls, allowing multiple mints in a single transaction and potentially bypassing supply or payment assumptions.

// Root cause in the codebase with @> marks to highlight the relevant section
​
​
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);
}

Risk

Likelihood:

  • Reason 1: Occurs when a caller’s usdc.transferFrom or the ERC721 receiver hook executes external code during mintNft() and that external code reenters the contract before the original call finishes.

  • Reason 2: Occurs when mintNft() is called by a contract that implements onERC721Received, so _safeMint triggers a callback into attacker‑controlled code before the function completes.

Impact:

  • ASupply bypass: single transaction can mint multiple NFTs, potentially exceeding MAX_SUPPLY or the intended one‑mint‑per‑call behavior.

  • ** **Colatteral/accounting corruption: The collateral or internal counters (like tokenIdCounter) can be advanced unexpectedly, making later state inconsistent or enabling under‑collateralized mints

Proof of Concept

/*
Tools used
- Foundry (forge test)
- Slither (via trailofbits/eth-security-toolbox Docker)
- Mythril (via mythril/myth Docker)
- Solc 0.8.34 (manual download in WSL)
​
PoC code created
- test/ReentrancyExploit.t.sol
- ReentrantUSDC (malicious ERC20 to reenter during transferFrom)
- ReentrantBuyer (ERC721 receiver reentering during _safeTransfer)
- testReentrancy_MintNft_BypassMaxSupply
- testReentrancy_Buy_ReenterWindow
*/
​

Recommended Mitigation

​
Based on OWASP SC05 (Reentrancy Attacks):
https://owasp.org/www-project-smart-contract-top-10/2025/en/src/SC05-reentrancy-attacks.html
​
- require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
- tokenIdCounter++;
- collateralForMinting[tokenIdCounter] = lockAmount;
- _safeMint(msg.sender, tokenIdCounter);
+ tokenIdCounter++;
+ collateralForMinting[tokenIdCounter] = lockAmount;
+ require(usdc.transferFrom(msg.sender, address(this), lockAmount), "USDC transfer failed");
+ _safeMint(msg.sender, tokenIdCounter);
+ // add ReentrancyGuard + nonReentrant on mintNft()
​
- bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
- require(success, "USDC transfer failed");
- _safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
- s_listings[_listingId].isActive = false;
+ s_listings[_listingId].isActive = false;
+ bool success = usdc.transferFrom(msg.sender, address(this), listing.price);
+ require(success, "USDC transfer failed");
+ _safeTransfer(listing.seller, msg.sender, listing.tokenId, "");
+ // add ReentrancyGuard + nonReentrant on buy()

Support

FAQs

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

Give us feedback!