code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

`VerbsToken.tokenURI` does not comply with ERC-721 specification #660

Closed c4-bot-10 closed 9 months ago

c4-bot-10 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/VerbsToken.sol#L194

Vulnerability details

According to the documentation VerbsToken: Should comply with ERC721.

The specification of ERC-721 states the following regarding the tokenURI function: Throws if _tokenId is not a valid NFT.

The tokenURI function in VerbsToken does not check if the _tokenId is a valid NFT, and therefore it is possible to call the function with an invalid _tokenId and get a response.

Impact

The Verbs token contract is not compliant with the ERC-721 specification.

Tools Used

Manual inspection.

Recommended Mitigation Steps

    function tokenURI(uint256 tokenId) public view override returns (string memory) {
+       require(uint8(artPieces[tokenId].metadata.mediaType) > 0, "Invalid tokenId");
        return descriptor.tokenURI(tokenId, artPieces[tokenId].metadata);
    }

Assessed type

ERC721

c4-pre-sort commented 10 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #110

c4-judge commented 9 months ago

MarioPoneder changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

MarioPoneder marked the issue as grade-b

c4-judge commented 9 months ago

This previously downgraded issue has been upgraded by MarioPoneder

c4-judge commented 9 months ago

MarioPoneder marked the issue as satisfactory