code-423n4 / 2023-04-caviar-findings

9 stars 4 forks source link

No Control for `tokenURI`, attributes, and `svg` Functions. #785

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L17-L28 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L17-L31 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L35-L51 https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L55-L93

Vulnerability details

Impact

Absence of access control for the tokenURI, attributes, and svg functions can have a significant impact on the security of the smart contract. Without any access control, any user can read the metadata for a private pool, which could reveal sensitive information such as the pool's token ID, base token, NFT, virtual base token reserves, virtual NFT reserves, fee rate, NFT balance, base token balance, and pool address. Furthermore, this could allow an attacker to manipulate the metadata, leading to unexpected behavior or even loss of funds.

Proof of Concept

The tokenURI function generates an NFT metadata JSON string that contains an SVG image and attributes of a private pool. The line of code that is vulnerable is the call to svg(tokenId) inside the metadata variable, this function generates an SVG image for the private pool based on its tokenId. However, there is no access control implemented, so anyone can call this function and retrieve the SVG image. The attributes function also contains sensitive information about the private pool that should only be accessed by authorized parties.

    function tokenURI(uint256 tokenId) public view returns (string memory) {
        // forgefmt: disable-next-item
        bytes memory metadata = abi.encodePacked(
            "{",
                '"name": "Private Pool ',Strings.toString(tokenId),'",',
                '"description": "Caviar private pool AMM position.",',
                '"image": ','"data:image/svg+xml;base64,', Base64.encode(svg(tokenId)),'",',
                '"attributes": [',
                    attributes(tokenId),
                "]",
            "}"
        );

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

We can attest the contract does not implement access control, which allows anyone to call the tokenURI(), attributes(), and svg() functions that should only be accessed by authorized parties by calling these functions from a different address than the authorized one.

Code block for tokenURI() function:

    function tokenURI(uint256 tokenId) public view returns (string memory) {
        // forgefmt: disable-next-item
        bytes memory metadata = abi.encodePacked(
            "{",
                '"name": "Private Pool ',Strings.toString(tokenId),'",',
                '"description": "Caviar private pool AMM position.",',
                '"image": ','"data:image/svg+xml;base64,', Base64.encode(svg(tokenId)),'",',
                '"attributes": [',
                    attributes(tokenId),
                "]",
            "}"
        );

        return string(abi.encodePacked("data:application/json;base64,", Base64.encode(metadata)));
    }

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

Code block for attributes() function:

    function attributes(uint256 tokenId) public view returns (string memory) {
        PrivatePool privatePool = PrivatePool(payable(address(uint160(tokenId))));

        // forgefmt: disable-next-item
        bytes memory _attributes = abi.encodePacked(
            trait("Pool address", Strings.toHexString(address(privatePool))), ',',
            trait("Base token", Strings.toHexString(privatePool.baseToken())), ',',
            trait("NFT", Strings.toHexString(privatePool.nft())), ',',
            trait("Virtual base token reserves",Strings.toString(privatePool.virtualBaseTokenReserves())), ',',
            trait("Virtual NFT reserves", Strings.toString(privatePool.virtualNftReserves())), ',',
            trait("Fee rate (bps): ", Strings.toString(privatePool.feeRate())), ',',
            trait("NFT balance", Strings.toString(ERC721(privatePool.nft()).balanceOf(address(privatePool)))), ',',
            trait("Base token balance",  Strings.toString(privatePool.baseToken() == address(0) ? address(privatePool).balance : ERC20(privatePool.baseToken()).balanceOf(address(privatePool))))
        );

        return string(_attributes);
    }

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L35-L51

Code block for svg() function:

    function svg(uint256 tokenId) public view returns (bytes memory) {
        PrivatePool privatePool = PrivatePool(payable(address(uint160(tokenId))));

        // break up svg building into multiple scopes to avoid stack too deep errors
        bytes memory _svg;
        {
            // forgefmt: disable-next-item
            _svg = abi.encodePacked(
                '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 400 400" style="width:100%;background:black;fill:white;font-family:serif;">',
                    '<text x="24px" y="24px" font-size="12">',
                        "Caviar AMM private pool position",
                    "</text>",
                    '<text x="24px" y="48px" font-size="12">',
                        "Private pool: ", Strings.toHexString(address(privatePool)),
                    "</text>",
                    '<text x="24px" y="72px" font-size="12">',
                        "Base token: ", Strings.toHexString(privatePool.baseToken()),
                    "</text>",
                    '<text x="24px" y="96px" font-size="12">',
                        "NFT: ", Strings.toHexString(privatePool.nft()),
                    "</text>"
            );
        }

        {
            // forgefmt: disable-next-item
            _svg = abi.encodePacked(
                _svg,
                '<text x="24px" y="120px" font-size="12">',
                    "Virtual base token reserves: ", Strings.toString(privatePool.virtualBaseTokenReserves()),
                "</text>",
                '<text x="24px" y="144px" font-size="12">',
                    "Virtual NFT reserves: ", Strings.toString(privatePool.virtualNftReserves()),
                "</text>",
                '<text x="24px" y="168px" font-size="12">',
                    "Fee rate (bps): ", Strings.toString(privatePool.feeRate()),
                "</text>"
            );
        }

https://github.com/code-423n4/2023-04-caviar/blob/cd8a92667bcb6657f70657183769c244d04c015c/src/PrivatePoolMetadata.sol#L55-L93

In the PrivatePoolMetadata contract, the tokenURI(), attributes(), and svg() functions are defined as public functions, which means that anyone can call them. However, these functions are intended to be accessed only by authorized parties. The lack of access control in these functions makes them vulnerable to exploitation.

Example scenario:

  1. A user wants to view the metadata of a private pool with a token ID of 123.
  2. The user calls the tokenURI() function with the tokenId parameter set to 123.
  3. The user receives the metadata for the private pool, including its attributes and SVG, even though they are not the owner or authorized to view the data.

Possible attack vectors:

  1. An attacker could call the tokenURI function for a private pool that they are not authorized to see, and view sensitive information about the pool.
  2. An attacker could modify the metadata for a private pool by calling the tokenURI function and changing the JSON-encoded attributes.
  3. An attacker could call the attributes function for a private pool that they are not authorized to see, and view sensitive information about the pool.
  4. An attacker could call the svg function for a private pool that they are not authorized to see, and view sensitive information about the pool.

Instance scenario:

Alternatively, Eve could modify the metadata for the private pool by changing the JSON-encoded attributes. This could misrepresent the information about the pool and potentially trick Bob into making a bad trade.

Tools Used

vs code

Recommended Mitigation Steps

Consider implementing access control in the PrivatePoolMetadata contract to restrict access to authorized parties only such as require(msg.sender == owner) or require(hasRole("admin", msg.sender)) should be implemented to restrict the access of the tokenURI(), attributes(), and svg() functions to only authorized parties.

function tokenURI(uint256 tokenId) public view returns (string memory) {
    require(msg.sender == ownerOf(tokenId), "Only the owner can access the token URI");
    ...
}
c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

outdoteth commented 1 year ago

All of this information is public anyway. its not an issue that somebody can read it

c4-sponsor commented 1 year ago

outdoteth marked the issue as sponsor disputed

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid