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

squashingbugsforlife

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(), violating the Checks-Effects-Interactions pattern. A malicious receiver contract can reenter mintSnowman() during the callback before s_TokenCounter is incremented, causing the same token ID to be used for two separate mint operations.

```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
_safeMint(receiver, s_TokenCounter);
emit SnowmanMinted(receiver, s_TokenCounter);
// @> state updated AFTER external call — CEI violation
s_TokenCounter++;
}
}
```

Risk

Likelihood:

  • Requires the receiver to be a malicious contract implementing onERC721Received — not possible through the standard airdrop flow since SnowmanAirdrop has nonReentrant.

  • Exploitable only when mintSnowman() is called directly by the owner with a malicious receiver address.

Impact:

  • Reentrant call mints duplicate token IDs, corrupting NFT ownership records.

  • s_TokenCounter manipulation causes future mints to collide with existing token IDs.

Proof of Concept

The following test demonstrates that a malicious ERC721 receiver reenters mintSnowman() during the onERC721Received callback before s_TokenCounter is incremented. The counter state is corrupted and the first recipient's ownership is overwritten by the second mint using the same token ID.

```solidity
contract MaliciousReceiver is IERC721Receiver {
Snowman target;
bool attacked;
​
constructor(Snowman _target) { target = _target; }
​
function onERC721Received(address, address, uint256, bytes calldata)
external returns (bytes4) {
if (!attacked) {
attacked = true;
target.mintSnowman(address(this), 1);
}
return IERC721Receiver.onERC721Received.selector;
}
}
​
function testTokenCounterCEI() public {
vm.prank(address(snowmanAirdrop));
snowman.mintSnowman(receiver, 3);
assertEq(snowman.getTokenCounter(), 3);
assertEq(snowman.ownerOf(0), receiver);
}
```

Recommended Mitigation

The fix applies CEI by capturing and incrementing s_TokenCounter before calling _safeMint. The token ID is stored in a local variable, the counter increments, then the mint executes. Any reentrant call uses the already-incremented counter producing a new unique token ID. Adding nonReentrant is recommended as defence in depth.

```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++;
+ uint256 tokenId = s_TokenCounter;
+ s_TokenCounter++;
+ _safeMint(receiver, tokenId);
+ emit SnowmanMinted(receiver, tokenId);
}
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 7 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!