code-423n4 / 2024-06-panoptic-findings

1 stars 0 forks source link

`FactoryNFT#tokenURI()` does not comply with 721 since it doedne check if the tokenId is valid #24

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/FactoryNFT.sol#L39-L51

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/FactoryNFT.sol#L39-L51


    function tokenURI(uint256 tokenId) public view override returns (string memory) {
        address panopticPool = address(uint160(tokenId));

        return
            constructMetadata(
                panopticPool,
                PanopticMath.safeERC20Symbol(PanopticPool(panopticPool).univ3pool().token0()),
                PanopticMath.safeERC20Symbol(PanopticPool(panopticPool).univ3pool().token1()),
                PanopticPool(panopticPool).univ3pool().fee()
            );
    }

According to the EIP, the tokenURI is expected to revert/throw in the case where the tokenId is not a valid one, see https://eips.ethereum.org/EIPS/eip-721

/// @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) external view returns (string);
}

Impact

Borderline low, medium, no QA so attaching as med.

Recommended Mitigation Steps

Consider ensuring that the tokenId is valid so as to conform with the EIP.

Assessed type

Context

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Out of scope

Picodes commented 1 month ago

Out of scope with the precision on the scope of the readme