LlamaPolicyMetadata.contractURI() can return corrupted JSON data

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-llama/blob/aac904d31639c1b4b4e97f1c76b9c0f40b8e5cee/src/LlamaPolicyMetadata.sol#L109

Vulnerability details

LlamaPolicyMetadata.contractURI() returns the contract URI for a given Llama policy, which gives the JSON data with the bio and generated svg image.

File: LlamaPolicyMetadata.sol
104: function contractURI(string memory name) public pure returns (string memory) {
105:     string[5] memory parts;
106:     parts[0] = '{ "name": "Llama Policies: ';
107:     parts[1] = name;
108:     parts[2] = '", "description": "This collection includes all members of the Llama organization: ';
109:     parts[3] = name;
110:     parts[4] =
111:       '. Visit https://app.llama.xyz to learn more.", "image":"https://llama.xyz/policy-nft/llama-profile.png", "external_link": "https://app.llama.xyz", "banner":"https://llama.xyz/policy-nft/llama-banner.png" }';
112:     string memory json = Base64.encode(bytes(string.concat(parts[0], parts[1], parts[2], parts[3], parts[4])));
113:     return string.concat("data:application/json;base64,", json);
114:   }

There is no checks on name upon deployment. If it contains double-quotes ", contractURI() will return a corrupted string.

Impact

If the name string has " character, it can either:

• lead to invalid JSON data that can lead to errors when parsed by an external application
• lead to overwriting of the "name": "Llama Policies: and "description": "This collection includes all members of the Llama organization:" properties of the contractURI. For instance, by using the following name string: ""name": "New Policy", "description": "new description"", the name and description properties parsed from the JSON object would be "name": "New Policy", "description": "new description", as opposed to the "Llama Policies:..." and "description": "This collection includes all members of the Llama organization:..." expected.
  (Note: this depends on what JSON parsing method is used, but in general the handling of key-value pair will overwrite the value if the same key is encountered multiple times.).

Tools Used

Manual Analysis

Recommended Mitigation Steps

Use the escapeJSON function from LibString to prevent the issue described above.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

AustinGreen marked the issue as sponsor acknowledged

[c4-sponsor]

AustinGreen marked the issue as disagree with severity

[AustinGreen]

Acknowledge but this should be of informational severity. It is extremely unlikely and there is no risk to Llama users.

[c4-judge]

gzeon-c4 changed the severity to QA (Quality Assurance)

[c4-judge]

gzeon-c4 marked the issue as grade-c

[c4-judge]

gzeon-c4 marked the issue as grade-b