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

1 stars 0 forks source link

JSON injection and xss through ERC20 symbol when generating `tokenUri` #4

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#L46-L47 https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/FactoryNFT.sol#L69-L112

Vulnerability details

Impact

The tokenURI function uses safeERC20Symbol function in panoptic math to get token symbols but doesn't sanitize it. This causes that any special charcacters can be introduced into the factory NFTs token uri leading to a host of JSON injection attack vectors. The use of SVG is well known to be often vulnerable to Cross-Site Scripting (XSS). If a malicious user can inject malicious JavaScript into an SVG file, any user viewing the SVG on a website may become susceptible to XSS attacks. Considering that anyone can create an ERC20 token, a uniswap pool and a panoptic pool or use the token symbol includes special characters, this can be weaponized by attackers to execute malicious codes on the frontend for instance, running a keylogger script to collect all inputs typed by a user including his password or to create a fake Metamask pop up asking a user to sign a malicious transaction to steal his funds in users. Even while the front end processes securely, such as using the standard builtin JSON.parse() to read URI. This can also be exploited by replacing factory nft 's svg with arbitrary other ones such as creating NFTs containing same art piece data with existing high price NFTs, or other legally risky svgs like gore or pornography images. More about this can be read here, here and here

Proof of Concept

When constructing metadata for the tokenUri, the metadata is constructed first getting the uniswap pools' token symbols. It does this using the safeERC20Symbol function.

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

The safeERC20Symbol function simply queries the symbol without actually sanitizing the returned symbols. No check for symbol length, and no check for special characters.

    function safeERC20Symbol(address token) external view returns (string memory) {
        // not guaranteed that token supports metadata extension
        // so we need to let call fail and return placeholder if not
        try IERC20Metadata(token).symbol() returns (string memory symbol) {
            return symbol;
        } catch {
            return "???";
        }
    }

This is then used to construct the metadata and to generate the svg info. Notice how the characters are still not sanitized

function constructMetadata(
        address panopticPool,
        string memory symbol0,
        string memory symbol1,
        uint256 fee
    ) public view returns (string memory) {
        uint256 lastCharVal = uint160(panopticPool) & 0xF;
        uint256 rarity = PanopticMath.numberOfLeadingHexZeros(panopticPool);

        string memory svgOut = generateSVGArt(lastCharVal, rarity);

        svgOut = generateSVGInfo(svgOut, panopticPool, rarity, symbol0, symbol1);
        return
            string(
                abi.encodePacked(
                    "data:application/json;base64,",
                    Base64.encode(
                        bytes(
                            abi.encodePacked(
                                '{"name":"',
                                abi.encodePacked(
                                    LibString.toHexString(uint256(uint160(panopticPool)), 20),
                                    "-",
                                    string.concat(
                                        metadata[bytes32("strategies")][lastCharVal].dataStr(),
                                        "-",
                                        LibString.toString(rarity)
                                    )
                                ),
                                '", "description":"',
                                string.concat(
                                    "Panoptic Pool for the ",
                                    symbol0,
                                    "-",
                                    symbol1,
                                    "-",
                                    PanopticMath.uniswapFeeToString(uint24(fee)),
                                    " market"
                                ),
                                '", "attributes": [{',
                                '"trait_type": "Rarity", "value": "',
                                string.concat(
                                    LibString.toString(rarity),
                                    " - ",
                                    metadata[bytes32("rarities")][rarity].dataStr()
                                ),
                                '"}, {"trait_type": "Strategy", "value": "',
                                metadata[bytes32("strategies")][lastCharVal].dataStr(),
                                '"}, {"trait_type": "ChainId", "value": "',
                                getChainName(),
                                '"}]',
                                '", "image": "',
                                "data:image/svg+xml;base64,",
                                Base64.encode(bytes(svgOut)),
                                '"}'
                            )
                        )
                    )
                )
            );
    }

This is dangerous as it can be weaponized to insert extra arbitrary data altering the integrity of the JSON data.

Tools Used

Manual code review

Recommended Mitigation Steps

Sanitize input data according: https://github.com/OWASP/json-sanitizer

Assessed type

Invalid Validation

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 scoping Q&A