hats-finance / Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad

1 stars 2 forks source link

OTAP contract doesn't comply with EIP-721 standard #10

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x6af5da7e85f39fbfdaea11f821b757bf021a10b3d29d67755bf2a338f770d924 Severity: medium

Description: Description\ According to the EIP-721 standard, the tokenURI function must revert if a non-existent tokenId is passed. In OTAP contract, this requirement is ignored. This leads to violation of the EIP-712 spec

OTAP.sol: 

    function tokenURI(uint256 tokenId) public view override(ERC721, ERC721NftLoader) returns (string memory) {
        return ERC721NftLoader.tokenURI(tokenId);
    }

ERC721NftLoader.sol:

/**
     * @notice Returns the token URI for a given token ID. If the NFT loader contract is not set, it returns an empty string.
     * @inheritdoc ERC721
     */
    function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
        if (address(nftLoader) == address(0)) {
            return "";
        }
        return INFTLoader(nftLoader).tokenURI(tokenId);
    }

Similar issue can be seen here: https://github.com/code-423n4/2023-04-caviar-findings/issues/44 https://github.com/code-423n4/2023-10-opendollar-findings/issues/243

Recommendation\ Add a token existence check, to follow EIP-721

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

0xRektora commented 4 weeks ago

Same answer as in https://github.com/hats-finance/Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad/issues/9#issuecomment-2147593322