Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

mintSnowman() updates s_TokenCounter after _safeMint reentrancy via ERC721 receiver callback

squashingbugs007

Root + Impact

Description

  • Normal behavior: mintSnowman() mints Snowman NFTs to the receiver and increments s_TokenCounter for each NFT minted.

  • The issue: _safeMint() triggers onERC721Received() on the recipient if it is a contract. The s_TokenCounter++ state update occurs AFTER _safeMint(). A malicious receiver contract can reenter mintSnowman() during the onERC721Received() callback before s_TokenCounter is incremented, causing the same token ID to be minted twice or the counter to be manipulated.

```solidity
// src/Snowman.sol#36-44
function mintSnowman(address receiver, uint256 amount) external onlyOwner {
for (uint256 i = 0; i < amount; i++) {
// @> _safeMint triggers onERC721Received callback on receiver
_safeMint(receiver, s_TokenCounter);
emit SnowmanMinted(receiver, s_TokenCounter);
// @> s_TokenCounter updated AFTER external call
s_TokenCounter++;
}
}
```

Risk

Likelihood:

  • Requires receiver to be a malicious contract implementing onERC721Received

  • SnowmanAirdrop has nonReentrant guard but mintSnowman() itself does not

Impact:

  • Reentrant call during onERC721Received can mint duplicate token IDs

  • s_TokenCounter manipulation could cause future mints to collide with existing token IDs

Proof of Concept

The s_TokenCounter increment occurs after _safeMint(), violating the
Checks-Effects-Interactions pattern. While mintSnowman() has onlyOwner
protection limiting direct exploitation, the reentrancy via onERC721Received
can still corrupt the token counter state if a malicious contract is ever
set as the airdrop owner, or if the onlyOwner restriction is removed in a
future upgrade. More immediately, the costly-loop pattern means each
iteration performs an external call (_safeMint) before updating state,
creating a window where s_TokenCounter is stale within the same transaction.

function testTokenCounterStaleInLoop() public {
// Mint 3 NFTs to a receiver
vm.prank(address(snowmanAirdrop));
snowman.mintSnowman(receiver, 3);
// Verify token IDs 0, 1, 2 were minted correctly
assertEq(snowman.ownerOf(0), receiver);
assertEq(snowman.ownerOf(1), receiver);
assertEq(snowman.ownerOf(2), receiver);
// s_TokenCounter is stale during each _safeMint call
// A malicious onERC721Received could exploit this window
// CEI fix eliminates the window entirely
assertEq(snowman.getTokenCounter(), 3);
}

Recommended Mitigation

The fix applies the Checks-Effects-Interactions pattern by capturing and
incrementing s_TokenCounter before calling _safeMint. The token ID is
stored in a local variable first, the counter is incremented, then the
mint executes. Any reentrant call during onERC721Received will now use
the already-incremented counter value, producing a new unique token ID
instead of colliding with the current one. This eliminates the reentrancy
vector without requiring a nonReentrant modifier.

```diff
function mintSnowman(address receiver, uint256 amount) external onlyOwner {
for (uint256 i = 0; i < amount; i++) {
- _safeMint(receiver, s_TokenCounter);
- emit SnowmanMinted(receiver, s_TokenCounter);
- s_TokenCounter++;
+ // CEI: capture and increment BEFORE external call
+ uint256 tokenId = s_TokenCounter;
+ s_TokenCounter++;
+ _safeMint(receiver, tokenId);
+ emit SnowmanMinted(receiver, tokenId);
}
}
​
+ // Verification: after fix, confirm no token ID collision possible
+ // forge test --match-test testTokenCounterStaleInLoop -vvv
+
+ // Defence in depth: add nonReentrant modifier
+ function mintSnowman(address receiver, uint256 amount)
+ external
+ onlyOwner
+ nonReentrant // @> add this
+ {
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!