code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/Factory.sol#L161 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L17

Vulnerability details

Impact

By invoking the Factory.tokenURI method for a maliciously provided NFT id, the returned data may deceive potential users, as the method will return data for a non-existent NFT id that appears to be a genuine PrivatePool. This can lead to a poor user experience or financial loss for users.
Violation of the ERC721-Metadata part standard

Proof of Concept

The Factory.tokenURI and PrivatePoolMetadata.tokenURI methods lack any requirements stating that the provided NFT id must be created. We can also see that in the standard implementation by OpenZeppelin, this check is present:
Throws if _tokenId is not a valid NFT

Example

User creates a fake contract A simple example so that the tokenURI method does not revert:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

contract NFT {
    function balanceOf(address) external pure returns (uint256) {
        1;
    }
}

contract NonNFT {
    address public immutable nft;

    address public constant baseToken = address(0);
    uint256 public constant virtualBaseTokenReserves = 1 ether;
    uint256 public constant virtualNftReserves = 1 ether;
    uint256 public constant feeRate = 500;

    constructor() {
        nft = address(new NFT());
    }
}