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

3 stars 3 forks source link

tokenURI function does not check if NFT exist #161

Closed c4-bot-10 closed 10 months ago

c4-bot-10 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-12-autonolas/blob/main/registries/contracts/GenericRegistry.sol#L135

Vulnerability details

Impact

ERC721 standard requires that the tokenURI function should revert in case the not existing id is specified, however current implementation of the GenericRegistry#tokenURI would successfully return results for NFTs that are not yet minted.

Proof of Concept

File: GenericRegistry.sol
130: 
131:     /// @dev Returns unit token URI.
132:     /// @notice Expected multicodec: dag-pb; hashing function: sha2-256, with base16 encoding and leading CID_PREFIX removed.
133:     /// @param unitId Unit Id.
134:     /// @return Unit token URI string.
135:     function tokenURI(uint256 unitId) public view virtual override returns (string memory) {
136:         bytes32 unitHash = _getUnitHash(unitId);
137:         // Parse 2 parts of bytes32 into left and right hex16 representation, and concatenate into string
138:         // adding the base URI and a cid prefix for the full base16 multibase prefix IPFS hash representation
139:         return string(abi.encodePacked(baseURI, CID_PREFIX, _toHex16(bytes16(unitHash)),
140:             _toHex16(bytes16(unitHash << 128))));
141:     }

Recommended Mitigation Steps

Consider adding a check to the tokenURI function that NFT with the specified ID exists.

Assessed type

ERC721

c4-pre-sort commented 10 months ago

alex-ppg marked the issue as duplicate of #344

c4-pre-sort commented 10 months ago

alex-ppg marked the issue as insufficient quality report

c4-judge commented 10 months ago

dmvt marked the issue as unsatisfactory: Out of scope