Snowman Merkle Airdrop
First Flight #42
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

[H-1] Multiple Security Flaws in `Snowman::mintSnowman`

0xsnoweth

Root + Impact

Root: The mintSnowman function lacks an onlyOwner modifier and violates CEI by minting before state updates.

Impact: Allows unauthorized minting, enables reentrancy attacks.

Description

The mintSnowman function is designed to mint a specified number of Snowman NFTs to a given receiver address by looping through _safeMint and incrementing a token counter.
The function lacks an onlyOwner modifier, allowing any address to mint NFTs and violates the Checks-Effects-Interactions (CEI) pattern by minting before updating the state.

// Root cause in the codebase with @> marks to highlight the relevant section
function mintSnowman(address receiver, uint256 amount) external {
@> for (uint256 i = 0; i < amount; i++) {
@> _safeMint(receiver, s_TokenCounter);
@> s_TokenCounter++;
@> }
}

Risk

Likelihood:

During any transaction where an external address calls mintSnowman.
When a malicious contract exploits the CEI order to re-enter the function.

Impact:

Unauthorized users can mint unlimited NFTs, draining the contract’s minting capacity.
Reentrancy could allow an attacker to mint excessive tokens, disrupting the token supply and potentially crashing the contract.

Proof of concept:

Missing onlyOwner Test: A non-owner address (nasar) successfully mints 10 NFTs, proving anyone can call the function.

function testMintingtheSnowman() public {
address nasar = makeAddr("nasar");
vm.prank(nasar);
uint256 initialTokenCounted = nft.getTokenCounter();
nft.mintSnowman(address(nasar), 10);
uint256 FinalTokenCounted = nft.getTokenCounter();
assertGt(FinalTokenCounted, initialTokenCounted);
assertEq(FinalTokenCounted, initialTokenCounted + 10);
console2.log("Non-reentrant mint count:", FinalTokenCounted);
}

  • CEI Violation: The function’s order _safeMint before s_TokenCounter++ poses a reentrancy risk, though not fully tested here due to potential validation concerns.

Recommended Mitigation

// 1. Add an OnlyOwner modifer or add the neccesary require/revert checks required to sustain function Ownership.
// 2. Add a NonReentrant modifer from Openzeppelin to avoid any sort of Reentrancy and use the correct CEI format.
​
​
import { ReentrancyGuard, Ownable } from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
​
contract Snowman is ReentrancyGuard, Ownable {
error OnlyOwnerAllowedForAccess();
​
function mintSnowman(address receiver, uint256 amount) external onlyOwner nonReentrant {
- for (uint256 i = 0; i < amount; i++) {
- _safeMint(receiver, s_TokenCounter);
- s_TokenCounter++;
- }
+ uint256 initialCounter = s_TokenCounter;
+ uint256 endCounter = initialCounter + amount;
+ s_TokenCounter = endCounter; // State update
+ emit SnowmanMinted(receiver, initialCounter, endCounter); // Event with range
+ for (uint256 i = initialCounter; i < endCounter; i++) {
+ _safeMint(receiver, i); // Mint with unique IDs
+ }
}

  • Add an OnlyOwner modifer or add the neccesary require/revert checks required to sustain function Ownership.

  • Add a NonReentrant modifer from Openzeppelin to avoid any sort of Reentrancy and use the correct CEI format.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Unrestricted NFT mint function

The mint function of the Snowman contract is unprotected. Hence, anyone can call it and mint NFTs without necessarily partaking in the airdrop.

Support

FAQs

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