Root + Impact

Description

Normal behavior:
The tokenURI(uint256 tokenId) function in an ERC721 contract should return the metadata for a given token if it exists, and safely reject requests for non-existent tokens without causing unexpected reverts. This is important for NFT indexers, marketplaces, and frontends that loop over token IDs or pre-fetch metadata.

The problem:
In Snowman.sol, the tokenURI() function checks whether a token exists by calling ownerOf(tokenId). However, ownerOf() reverts if the token does not exist, instead of returning address(0). As a result, calling tokenURI() on an unminted token will revert, making the function behave unexpectedly and causing compatibility issues with external tools.

Risk

Likelihood: Medium

• This occurs during any metadata query for a token ID that has not been minted yet, which is common in frontends or services that iterate over token ranges or attempt pre-fetching.

• External dApps or services like indexers, wallets, or marketplaces often rely on tokenURI() to not revert unexpectedly, and will encounter this issue during routine token discovery.

Impact: Medium

• External tools such as marketplaces like OpenSea, NFT explorers, indexers, or frontend applications may revert unexpectedly when querying metadata for unminted token IDs, even during legitimate usage.

• Breaks expected ERC721 behavior for metadata access, leading to poor protocol integration, failed metadata lookups, and reduced trust in the contract's reliability for NFT applications.

Proof of Concept

The contract uses ownerOf(tokenId) to check whether a token exists in tokenURI(). This causes the function to revert on unminted token IDs, breaking expected metadata access behavior. It creates integration issues and makes the contract incompatible with NFT crawlers and indexers that rely on graceful failure handling.

Recommended Mitigation

The current tokenURI(uint256 tokenId) implementation uses ownerOf(tokenId) == address(0) to check whether a token exists. This approach is problematic because:

• ownerOf(tokenId) reverts if the token does not exist, causing the entire call to fail unexpectedly instead of returning a controlled error.

• Using ownerOf for existence checks can break frontend or other contract interactions relying on a graceful error or false response for non-existent tokens.

Replacing this check with the ERC721 internal function _exists(tokenId) provides a non-reverting, boolean check for token existence that:

• Returns false if the token does not exist.

• Enables controlled revert with a custom error (ERC721Metadata__URI_QueryFor_NonExistentToken()).

• Improves robustness and predictability of tokenURI calls.

• Aligns with best practices recommended by the OpenZeppelin ERC721 implementation.

Impact of Applying this Mitigation

• Prevents unexpected reverts from ownerOf during metadata queries.

• Enhances compatibility with wallets, marketplaces, and other consumers of the ERC721 metadata.

• Improves UX by ensuring token URI queries for non-existent tokens fail cleanly.