Root + Impact
Description
-
The mintSnowman function should validate input parameters to prevent unintended minting behaviour.
-
The function accepts address(0) as a valid receiver, even though sending tokens to this address effectively burns them as they can never be recovered. While OpenZeppelin's _safeMint includes some safety checks, it doesn't prevent minting to address(0). This is intentional in the OpenZeppelin implementation to allow flexibility, but it means the responsibility for this validation falls on the implementing contract. The lack of validation means that a simple frontend error or incorrect parameter passing could result in permanent loss of tokens.
@> 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++;
} @>
Risk
Likelihood: Low
-
Function can be called with zero address as receiver
-
No validation exists for the amount parameter
-
Occurs in any direct contract interaction where parameters aren't validated
Impact:
-
Tokens could be minted to the zero address, effectively burning them
-
Zero amount of mints waste gas but completes successfully
-
No maximum batch size limit could lead to gas issues
Proof of Concept
pragma solidity 0.8.24;
import {Test} from "forge-std/Test.sol";
import {Snowman} from "../src/Snowman.sol";
contract SnowmanTest is Test {
Snowman snowman;
address alice = makeAddr("alice");
function setUp() public {
snowman = new Snowman("test-uri");
}
function testMintToZeroAddress() public {
snowman.mintSnowman(address(0), 1);
assertEq(snowman.ownerOf(0), address(0));
snowman.mintSnowman(address(0), 3);
assertEq(snowman.ownerOf(1), address(0));
assertEq(snowman.ownerOf(2), address(0));
assertEq(snowman.ownerOf(3), address(0));
vm.prank(alice);
snowman.mintSnowman(alice, 0);
address unsetAddress;
snowman.mintSnowman(unsetAddress, 1);
}
}
Recommended Mitigation
// Add custom errors
- // ... existing code ...
+ error ZeroAddressNotAllowed();
+ error InvalidMintAmount();
// Add constants
+ uint256 public constant MAX_BATCH_SIZE = 20;
// Modify minting function
- 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++;
- }
- }
+ function mintSnowman(address receiver, uint256 amount) external {
+ // Validate receiver address
+ if (receiver == address(0)) revert ZeroAddressNotAllowed();
+
+ // Validate mint amount
+ if (amount == 0 || amount > MAX_BATCH_SIZE) revert InvalidMintAmount();
+
+ // Store starting token ID for event
+ uint256 startTokenId = s_TokenCounter;
+
+ // Perform minting
+ for (uint256 i = 0; i < amount; i++) {
+ _safeMint(receiver, s_TokenCounter);
+ s_TokenCounter++;
+ }
+
+ // Emit single event for batch mint
+ emit SnowmanMinted(receiver, startTokenId, amount);
+ }
