Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

`Snowman::tokenURI`'s nonexistent-token guard is unreachable dead code because OpenZeppelin v5 `ownerOf` reverts instead of returning the zero address, so callers receive a different error than the contract declares

friedsocks233

Description

tokenURI is meant to reject queries for tokens that do not exist, and the contract declares error ERC721Metadata__URI_QueryFor_NonExistentToken() for that purpose.

The guard compares ownerOf(tokenId) against the zero address. That pattern belongs to OpenZeppelin v4. In v5 — which this project uses — ownerOf routes through _requireOwned and reverts with ERC721NonexistentToken(tokenId) rather than returning address(0). Control therefore never reaches the comparison, the custom error can never be thrown, and the branch is dead code.

// src/Snowman.sol
@> error ERC721Metadata__URI_QueryFor_NonExistentToken();
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
@> if (ownerOf(tokenId) == address(0)) {
@> revert ERC721Metadata__URI_QueryFor_NonExistentToken();
}
string memory imageURI = s_SnowmanSvgUri;
...
// lib/openzeppelin-contracts/contracts/token/ERC721/ERC721.sol (v5)
function ownerOf(uint256 tokenId) public view virtual returns (address) {
return _requireOwned(tokenId); // reverts ERC721NonexistentToken(tokenId)
}

Risk

Likelihood:

  • On every tokenURI call for a nonexistent token — the one case the branch was written to handle.

Impact:

  • Callers and integrators receive ERC721NonexistentToken(uint256) while the contract's ABI advertises ERC721Metadata__URI_QueryFor_NonExistentToken(). Any front end or indexer that decodes the declared error will fail to match the revert.

  • Dead code and an unreachable error selector in a deployed contract: misleading to reviewers and wasteful of deployment gas. No funds are at risk.

Proof of Concept

Paste the harness and test function(s) into test/PoC.t.sol, placing test functions inside contract PoC. Any additional helper contracts go outside contract PoC. Run forge test --match-path test/PoC.t.sol -vv.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console2} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
import {Snowman} from "../src/Snowman.sol";
import {SnowmanAirdrop} from "../src/SnowmanAirdrop.sol";
import {MockWETH} from "../src/mock/MockWETH.sol";
contract PoC is Test {
Snow snow;
Snowman nft;
MockWETH weth;
address collector = makeAddr("collector");
address attacker = makeAddr("attacker");
address victim = makeAddr("victim");
function setUp() public {
weth = new MockWETH();
snow = new Snow(address(weth), 1, collector);
nft = new Snowman("ipfs://snowman");
}
function _leaf(address who, uint256 amount) internal pure returns (bytes32) {
return keccak256(bytes.concat(keccak256(abi.encode(who, amount))));
}
// Insert the test function below here.
}

Asserting that the OpenZeppelin error fires proves the contract's own branch is never evaluated.

function test_TokenUriCustomErrorIsDeadCode() public {
// Reverts inside OZ v5 ownerOf() via _requireOwned, BEFORE the contract's
// own zero-address branch is ever evaluated.
vm.expectRevert(abi.encodeWithSignature("ERC721NonexistentToken(uint256)", uint256(0)));
nft.tokenURI(0);
}

Recommended Mitigation

- error ERC721Metadata__URI_QueryFor_NonExistentToken();
error SM__NotAllowed();
...
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
- if (ownerOf(tokenId) == address(0)) {
- revert ERC721Metadata__URI_QueryFor_NonExistentToken();
- }
+ _requireOwned(tokenId);
+
string memory imageURI = s_SnowmanSvgUri;
Updates

Lead Judging Commences

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