DatingDapp

First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

tokenURI() reverts for burned tokens breaking metadata structure.

Summary

In SoulboundProfileNFT.sol, tokenURI calls ownerOf(tokenId), which reverts for burned tokens, making the metadata inaccessible.

Vulnerability Details

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

When a token is burned:

The burnProfilefunction deletes the profileToTokenand _profiles mapping entries, however if the same user tries to mint again the require(profileToToken[msg.sender] == 0, "Profile already exists");would pass because it is deleted. So they can mint a new profile

In the tokenURI, OpenZeppelin's ownerOf will revert with "ERC721: invalid token ID"

It will never return address(0)

Therefore, the check if (ownerOf(tokenId) == address(0)) can never be true

This means:

The function will always revert for burned tokens

Function execution will never reach the custom error ERC721Metadata__URI_QueryFor_NonExistentToken()

Impact

There is no way to distinguish between tokens that never existed and burned tokens

Lost historical metadata for burned tokens

Tools Used

Foundry

Recommendations

Implement OpenZeppelin's internal _ownerOffunction

function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
if (_ownerOf(tokenId) == address(0)) {
revert ERC721Metadata__URI_QueryFor_NonExistentToken();
}
// ... rest of the function

Or replace ownerOf(tokenId)with a custom check for token

Updates

Appeal created

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.