Snowman Merkle Airdrop
First Flight #42
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

Snowman::mintSnowman() contains risky loop for minting NFTs, a single bad item can fail whole transaction and fail to mint NFTs

anuragshingare2005

Snowman::mintSnowman() contains risky loop for minting NFTs, a single bad item can fail whole transaction and fail to mint NFTs

Description

  • Snowman::mintSnowman() mints amount of NFTs at receiver address.

  • NFTs are minted at receiver address by running a for loop 'amount' times

  • incrementing s_TokenCounter each time to ensure unique token IDs.

  • runs a loop amount of times to mint amount of NFTs, running a loop for minting NFTs can be risky such that a single bad item can fail whole transaction

contract Snowman is ERC721, Ownable {
// rest of code....
​
///@notice Function mints amount of NFT to the receiver address
// @audit-issue If receiver is contract -> and contract is malicious or broken -> Mint transaction will fail
// @audit-issue If receiver tries to mint a very large amount -> loop may consume more gas than allowed in a single block → causes Out of Gas error.
// @audit-issue Single bad item can fail whole transaction
function mintSnowman(address receiver, uint256 amount) external{
@> for (uint256 i = 0; i < amount; i++) {
_safeMint(receiver, s_TokenCounter);
​
emit SnowmanMinted(receiver, s_TokenCounter);
​
s_TokenCounter++;
}
}
​
// rest of functions.....
}

Risk

Likelihood:

  • If receiver is a contract and User mints more than 100 NFTs, and the 100th mint fails due to onERC721Received() on receiver's contract side, none of the 99 NFTs before that will be minted either.

  • If receiveris an broken or malicious contract, it can Revert unexpectedly, Fail to return the required selector .

Impact:

  • Whole transaction can failed due to a single bad item or bad transaction

  • It can dissrupt the s_TokenCounter to ensure the unique tokenId

Proof of Concept

  • The below test depects the unusal behaviour of Snowman::mintSnowman()

  • If receiver is an broken contract that does not implement onERC721Received() , can be the cause of error

// Below test depects that if receiver is an contract and it is broken or malicious
// minting NFTs can fail unexpectedly
​
​
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import "../src/Snowman.sol";
​
// Broken/Malicious contract that does NOT implement onERC721Received
contract BadReceiver {}
​
​
contract SnowmanTest is Test {
Snowman snowman;
BadReceiver badReceiver;
​
function setUp() public {
snowman = new Snowman("ipfs://snowman.svg");
badReceiver = new BadReceiver();
}
​
function testMintSnowmanFailsIfReceiverIsBadContract() public {
// Minting 3 NFTs from broken contract
vm.expectRevert(); // Expect revert due to _safeMint failure
snowman.mintSnowman(address(badReceiver), 3);
​
// 0 token is minted
assertEq(snowman.getTokenCounter(), 0);
}
}

Recommended Mitigation

  • Before calling _safeMint, check if the receiver is a smart contract, and validate it implements onERC721Received() correctly.

  • Add OpenZeppelins::ReentrancyGuard which can prevent the future Re-entrancy attack

+ Add this function to check receiver is an unbroken contract
@> function _isSafeReceiver(address receiver) internal view returns (bool) {
if (receiver.code.length == 0) return true; // Not a contract
​
try IERC721Receiver(receiver).onERC721Received(msg.sender, msg.sender, 0, "") returns (bytes4 retval) {
return retval == IERC721Receiver.onERC721Received.selector;
} catch {
return false;
}
@> }
​
+ Before minting NFTs add this require statement
@> require(_isSafeReceiver(receiver), "Receiver contract not ERC721 compatible");
​
+ Add OpenZeppelins::ReentrancyGuard
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract Snowman is ReentrancyGuard {
// Rest of code...
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
7 days ago
yeahchibyke Lead Judge 4 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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