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

Unreachable Code in tokenURI() Function

r7al38

Root + Impact

Description

  • In an ERC721 token, the tokenURI function is expected to return the metadata URI for a valid token. The standard practice is to first verify that the token exists, reverting with an appropriate error if it does not. Many implementations, including OpenZeppelin’s, use _exists or rely on ownerOf (which reverts on non‑existent tokens) to perform this check.

    In the Snowman contract, the tokenURI function attempts to validate existence by checking whether ownerOf(tokenId) == address(0). However, the OpenZeppelin ownerOf implementation does not return address(0) for non‑existent tokens; it reverts immediately. As a result, the condition ownerOf(tokenId) == address(0) can never evaluate to true, and the subsequent revert statement is unreachable.

function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
if (ownerOf(tokenId) == address(0)) { // @audit This condition is never true – ownerOf reverts instead of returning zero
revert ERC721Metadata__URI_QueryFor_NonExistentToken();
}
// ...
}

Risk

Likelihood:

  • The condition will never be satisfied in any execution path, because ownerOf always reverts for non‑existent tokens.

  • The dead code is permanently included in the deployed bytecode, regardless of how the function is used.

Impact:

  • Increased deployment cost – The superfluous code consumes gas when the contract is deployed.

  • No impact on runtime behaviour – The function still correctly reverts via ownerOf, but the custom error is never thrown.

Proof of Concept

The following Foundry test demonstrates that calling tokenURI with a non‑existent token ID reverts with the default OpenZeppelin error (ERC721NonexistentToken), not the custom one. This proves the custom revert path is never taken.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import "../src/Snowman.sol";
​
contract SnowmanTest is Test {
Snowman public snowman;
​
// fake address
address user = address(0x123);
​
error ERC721NonexistentToken(uint256 tokenId);
​
function setUp() public {
snowman = new Snowman("svg_example_uri");
}
​
function testOwnerOfRevertsForNonexistentToken() public {
uint256 nonexistentTokenId = 999;
​
vm.expectRevert(abi.encodeWithSelector(ERC721NonexistentToken.selector, nonexistentTokenId));
snowman.ownerOf(nonexistentTokenId);
}
​
function testTokenURIRevertsWithSameErrorAsOwnerOf() public {
uint256 nonexistentTokenId = 999;
vm.expectRevert(abi.encodeWithSelector(ERC721NonexistentToken.selector, nonexistentTokenId));
snowman.tokenURI(nonexistentTokenId);
}
}
​

Recommended Mitigation

Remove the unreachable code. If preserving the custom error message is desired, replace the check with the internal _exists function (inherited from ERC721).

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

Lead Judging Commences

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