Root + Impact

Description

• Normal Behavior: The mintSnowman function is designed to mint a single Snowman NFT per eligible claim and increment the global s_TokenCounter exactly once to ensure each NFT has a unique on-chain ID.

• Specific Issue: The contract violates the Checks-Effects-Interactions (CEI) pattern by performing an external call (_safeMint) before updating the internal state variable s_TokenCounter. Since _safeMint includes a mandatory callback to onERC721Received if the recipient is a contract, an attacker can re-enter the function before the counter increments.

Risk

• Reason 1: The receiver address is a user-supplied parameter, allowing any attacker to provide the address of a malicious contract specifically designed to exploit the callback.

• Reason 2: The use of _safeMint is a standard OpenZeppelin implementation that always attempts a hand-shake with contract recipients, making the attack vector a native feature of the ERC721 standard.

Impact:

• Impact 1 (Infinite Minting): Attackers can bypass intended limits to mint a much larger number of NFTs than their Snow token stake should allow.

• Impact 2 (Value Dilution): Uncontrolled inflation of the NFT supply destroys the rarity and market value of the Snowman collection for all legitimate holders.

Proof of Concept

Preparation: The attacker deploys a contract AttackSnowman that implements the IERC721Receiver interface.

Initial Call: The attacker calls mintSnowman through their contract.

The Hook: The Snowman contract executes _safeMint, which in turn calls AttackSnowman.onERC721Received.

The Re-entry: Inside the onERC721Received function, the attacker's contract calls Snowman.mintSnowman again.

State Stagnation: Because the first execution has not yet reached the s_TokenCounter++ line, the second execution sees the same counter value and bypasses any checks based on the current state.

Cycle: This process repeats until the attacker has minted the desired number of NFTs or the gas limit is reached.

Recommended Mitigation

Implementation

Apply the Checks-Effects-Interactions pattern by updating the state variable before making the external call. Alternatively, use OpenZeppelin's ReentrancyGuard.

Explanation
By incrementing s_TokenCounter before the _safeMint call, any subsequent re-entrant call will encounter a different (incremented) state. This ensures that even if a callback occurs, the "Effect" on the state is already finalized, preventing the logic from being tricked into using stale data.