Factory.sol#tokenURI() - The function doesn’t comply with ERC721 standard

Summary

The function doesn’t comply with ERC721 standard

Vulnerability Detail

tokenURI doesn’t follow [ERC721 standard](https://eips.ethereum.org/EIPS/eip-721#:~:text=function tokenURI(uint256 _tokenId) external view returns (string)%3B)

/// @notice A distinct Uniform Resource Identifier (URI) for a given asset.
/// @dev Throws if `_tokenId` is not a valid NFT. URIs are defined in RFC
///  3986. The URI may point to a JSON file that conforms to the "ERC721
///  Metadata JSON Schema".
function tokenURI(uint256 _tokenId)externalviewreturns (string);

If a id is not a valid id for an NFT (it doesn’t exist) the function should revert.

Currently this doesn’t happen.

function tokenURI(uint256 tokenId) public view override returns (string memory uri) {
        return bytes(baseURI).length > 0 ? string(abi.encodePacked(baseURI, tokenId.toString())) : "";
    }

The README of the contest states

Is the code/contract expected to comply with any EIPs? Are there specific assumptions around adhering to those EIPs that Watsons should be aware of?

• The Arcadia Accounts themselves are ERC721s (Factory maps id one-to-one to Account contract address).

Impact:

tokenURI will return token uri’s for bogus/fake/non-existing NFT’s (Accounts)

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/de7289bebb3729505a2462aa044b3960d8926d78/accounts-v2/src/Factory.sol#L335-L337

Tool used

Manual Review

Recommendation

Add an existence check inside tokenUri

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

nevillehuang commented 9 months ago

Invalid, same reasonings as #220

Low severity based on the following sherlock rules, since no funds are at risk and tokenURI() is simply a view function not used anywhere else in the protocol.

EIP Compliance: For issues related to EIP compliance, the protocol & codebase must show that there are important external integrations that would require strong compliance with the EIP's implemented in the code. The EIP must be in regular use or in the final state for EIP implementation issues to be considered valid

sherlock-audit / 2023-12-arcadia-judging

deth - Factory.sol#tokenURI() - The function doesn’t comply with ERC721 standard #209