Json injection

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L286-L305

Vulnerability details

Impact

The MetadataRenderer.tokenURI method is vulnerable to json injections.

Proof of Concept

Sanitize user inputs is mandatory because the user can inject into the json in order to construct a different one.

    function tokenURI(uint256 _tokenId) external view returns (string memory) {
        (bytes memory aryAttributes, bytes memory queryString) = getAttributes(_tokenId);
        return
            _encodeAsJson(
                abi.encodePacked(
                    '{"name": "',
                    settings.name,
                    " #",
                    LibUintToString.toString(_tokenId),
                    '", "description": "',
                    settings.description,
                    '", "image": "',
                    settings.rendererBase,
                    queryString,
                    '", "properties": {',
                    aryAttributes,
                    "}}"
                )
            );
    }

it is possible to take advantage of arguments such as settings.name or settings.description to inject json content and form a different content that can be used to advantage in a dApp.

Reference

• https://www.acunetix.com/blog/web-security-zone/what-are-json-injections/

Recommended Mitigation Steps

• Limit the length of the values or escape the string.

[GalloDaSballo]

I think this finding has validity and a warning should be put in the contract code.

However, React will render sanitized input (unless you use .eval , see this article https://www.whitehatsec.com/blog/handling-untrusted-json-safely/#:~:text=Parsing%20JSON%20can%20be%20a,will%20execute%20during%20parse%20time.)

All in all some validity but security should be provided by the renderer like all Frontends

As in: The contract is responsible for it's own protection (reEntrancy, overflows, wrong logic, etc..) The frontend is responsible for not allowing XSS, assuming the "server" is trustworthy is how you get rekt (Same deal for the server, the server must validate the frontend or contract data to protect itself)

[GalloDaSballo]

Because the vulnerability is dependent on an implementation from a frontend, which is not predictable, I will downgrade to Low Severity.

I highly recommend the Sponsor to make sure their frontend is not vulnerable to XSS, and recommend the Warden to check the live website and monitor for XSS as a way to get a new bounty once the site is live

[GalloDaSballo]

L