Incorrect Assumption in FactoryNFT Can Lead to Reverts During Token URI Retrieval.

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

tokenURI function assumes that the first 160 bits of the tokenId represent a valid PanopticPool address. However, it does not perform any validation to ensure that the address is indeed a valid PanopticPool contract.

If a tokenId is passed that does not correspond to a valid PanopticPool address, the function will still attempt to interact with the contract at that address. This can lead to even revert if the address does not implement the expected PanopticPool interface.

Proof of Concept

FactoryNFT contract (@contracts\base\FactoryNFT.sol:40-49)

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()
        );
}

As you can see, the function directly converts the first 160 bits of tokenId into an address using address(uint160(tokenId)). It then proceeds to interact with the contract at that address, assuming it is a valid PanopticPool contract.

Let's consider a scenario where a tokenId is passed that does not correspond to a valid PanopticPool address. For example:

uint256 invalidTokenId = 123456789; // An arbitrary token ID that does not represent a valid PanopticPool address

string memory metadata = factoryNFT.tokenURI(invalidTokenId);

In this case, the tokenURI function will attempt to interact with the contract at the address represented by invalidTokenId. If that address does not implement the PanopticPool interface or does not have the required functions, the function calls will revert, causing the transaction to fail.

Tools Used

Vs Code

Recommended Mitigation Steps

Add a validation check to ensure that the extracted address is a valid PanopticPool contract before interacting with it, can be done by adding a mapping or a whitelist of valid PanopticPool addresses and checking against it, or by using a factory pattern where only valid PanopticPool contracts are created and tracked.

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

    // Validate that the extracted address is a valid PanopticPool contract
    require(isValidPanopticPool(panopticPool), "Invalid PanopticPool address");

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

function isValidPanopticPool(address poolAddress) internal view returns (bool) {
    // Implement the validation logic here, e.g., check against a mapping or whitelist
    // Return true if the address is a valid PanopticPool contract, false otherwise
}

Assessed type

Invalid Validation

[c4-judge]

Picodes marked the issue as unsatisfactory: Out of scope

[Picodes]

Out of scope