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

tokenURI` ownership check is unreachable — `ownerOf()` reverts before `address(0)` comparison is evaluated

virgilbb

Root + Impact

Description

  • tokenURI() is intended to revert with a custom ERC721Metadata__URI_QueryFor_NonExistentToken error when queried for a non-existent token ID.

  • The check if (ownerOf(tokenId) == address(0)) is permanently unreachable. In OpenZeppelin ERC721 (v4 and v5), ownerOf() internally calls _ownerOf() and reverts with ERC721NonexistentToken if the token does not exist — it never returns address(0). For non-existent tokens, the revert fires before the comparison is evaluated; for existing tokens, the owner can never be address(0). The custom error is dead code.

function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
@> if (ownerOf(tokenId) == address(0)) {
revert ERC721Metadata__URI_QueryFor_NonExistentToken();
}
// ...
}

Risk

Likelihood:

  • The condition is unreachable on every call for every token ID — there is no code path that reaches the custom revert.

  • Integrators who write error-handling logic specifically for ERC721Metadata__URI_QueryFor_NonExistentToken discover it never fires during testing.

Impact:

  • No funds are at risk. The custom error is dead code — integrators who catch it will never see it, which can cause confusion during integration and testing.

  • Non-existent token queries still revert, just with OpenZeppelin's ERC721NonexistentToken instead of the custom error.

Proof of Concept

Place this test in test/ and run forge test --match-test testCustomErrorNeverEmitted. The test demonstrates that the custom NonExistentToken error in tokenURI() is unreachable because ownerOf() on a non-existent token reverts before the custom check is evaluated.

import {Test} from "forge-std/Test.sol";
import {Snowman} from "../src/Snowman.sol";
import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
​
contract UnreachableCheckPoC is Test {
Snowman snowman;
​
function setUp() public {
snowman = new Snowman("data:image/svg+xml;base64,PHN2Zy8+");
}
​
function testCustomErrorNeverEmitted() public {
uint256 nonExistentToken = 9999;
​
// This reverts with ERC721NonexistentToken (OZ), NOT the custom error
vm.expectRevert(
abi.encodeWithSelector(IERC721Errors.ERC721NonexistentToken.selector, nonExistentToken)
);
snowman.tokenURI(nonExistentToken);
// ERC721Metadata__URI_QueryFor_NonExistentToken is never emitted
}
}

Recommended Mitigation

Replace ownerOf(tokenId) == address(0) with _ownerOf(tokenId) == address(0) so the internal lookup returns zero instead of reverting, allowing the custom error to fire.

function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
- if (ownerOf(tokenId) == address(0)) {
+ if (_ownerOf(tokenId) == address(0)) {
revert ERC721Metadata__URI_QueryFor_NonExistentToken();
}
// ...
}
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!