Sablier

Sablier
DeFiFoundry
53,440 USDC
View results
Submission Details
Severity: medium
Valid

`SablierV2NFTDescriptor` is vulnerable to JSON Injection

Summary

The tokenURI function constructs a JSON object using user-provided data without proper sanitization. This makes the function vulnerable to JSON injection attacks, where malicious users can inject special characters to alter the JSON structure or inject malicious scripts.

Vulnerability Details

The issue exists in the tokenURI function, which constructs a JSON object using various pieces of data derived from user input or external contracts. These data points include strings that could potentially contain special characters.

Firstly, we have a User-Provided Data:
The function tokenURI uses several pieces of data that are derived from user input or external contracts. For example:

vars.assetSymbol = safeAssetSymbol(vars.asset);
vars.sablierModel = mapSymbol(sablier);
vars.status = stringifyStatus(vars.sablier.statusOf(streamId));

Then, the JSON metadata is constructed using these user-provided strings without any sanitization:

vars.json = string.concat(
'{"attributes":',
generateAttributes({
assetSymbol: vars.assetSymbol,
sender: vars.sablier.getSender(streamId).toHexString(),
status: vars.status
}),
',"description":"',
generateDescription({
sablierModel: vars.sablierModel,
assetSymbol: vars.assetSymbol,
sablierStringified: vars.sablierStringified,
assetAddress: vars.asset.toHexString(),
streamId: streamId.toString(),
isTransferable: vars.isTransferable
}),
'","external_url":"https://sablier.com","name":"',
generateName({ sablierModel: vars.sablierModel, streamId: streamId.toString() }),
'","image":"data:image/svg+xml;base64,',
Base64.encode(bytes(vars.svg)),
'"}'
);

If any of these strings contain special characters like ", : , or other JSON control characters, they could break the JSON structure or inject malicious data.

For example, let's consider where the asset symbol contains a double quote (") which is a special character in JSON.

Suppose the safeAssetSymbol function retrieves an asset symbol that includes a double quote: ABC"D. The safeAssetSymbol function does not sanitize this symbol, so it would be used as-is in the JSON metadata.

Here's how the tokenURI function would construct the JSON metadata with this unsanitized symbol:

{
"attributes": [
{
"trait_type": "Asset",
"value": "ABC"D" // Problematic part due to unescaped double quote
},
{
"trait_type": "Sender",
"value": "0x123"
},
{
"trait_type": "Status",
"value": "Streaming"
}
],
"description": "A description with special characters like \" and \\ and more.",
"external_url": "https://sablier.com",
"name": "Sablier V2 Lockup Linear #1",
"image": "data:image/svg+xml;base64,..."
}

We can see that the JSON is broken because of the unescaped double quote in the assetSymbol value. The JSON parser would interpret the second double quote in ABC"D as the end of the value string, causing a syntax error in the JSON. This demonstrates the potential for JSON injection vulnerabilities if special characters are not properly escaped.

One may want to argue that the safeAssetSymbol function already sanitizes the symbol, truncating it if it's too long or defaulting to "ERC20" if the call fails and also that the mapSymbol function maps known symbols to human-readable strings or reverts if unknown, avoiding injection.

But the thing is that the safeAssetSymbol and mapSymbol functions do provide some level of sanitization and validation, but they do not fully mitigate the JSON injection issue.

  • Let's look at the safeAssetSymbol Function:
    The safeAssetSymbol function attempts to retrieve the asset's symbol and provides some fallback mechanisms:

function safeAssetSymbol(address asset) internal view returns (string memory) {
(bool success, bytes memory returnData) = asset.staticcall(abi.encodeCall(IERC20Metadata.symbol, ()));
// Non-empty strings have a length greater than 64, and bytes32 has length 32.
if (!success || returnData.length <= 64) {
return "ERC20";
}
string memory symbol = abi.decode(returnData, (string));
// The length check is a precautionary measure to help mitigate potential security threats from malicious assets
// injecting scripts in the symbol string.
if (bytes(symbol).length > 30) {
return "Long Symbol";
} else {
return symbol;
}
}

Fallback Mechanism in the sense that if the call to retrieve the symbol fails or returns a short string, it defaults to "ERC20".
and a length check if the symbol is longer than 30 characters, it defaults to "Long Symbol". These helps to mitigate some risk but they do not fully sanitize the symbol. Special characters within the symbol are not escaped, which means they could still break the JSON structure.

  • Also let's look at the mapSymbol function:
    The mapSymbol function maps known symbols to human-readable strings or reverts if the symbol is unknown.

function mapSymbol(IERC721Metadata sablier) internal view returns (string memory) {
string memory symbol = sablier.symbol();
if (symbol.equal("SAB-V2-LOCKUP-LIN")) {
return "Lockup Linear";
} else if (symbol.equal("SAB-V2-LOCKUP-DYN")) {
return "Lockup Dynamic";
} else if (symbol.equal("SAB-V2-LOCKUP-TRA")) {
return "Lockup Tranched";
} else {
revert Errors.SablierV2NFTDescriptor_UnknownNFT(sablier, symbol);
}
}

This functions helps to ensure that only known symbols are used, which helps mitigate the risk of injection through the sablierModel field. However, it does not address other fields that may contain user-provided data.

The primary issue still exists because other fields in the JSON construction, such as description, name, and status, are not sanitized. These fields can still contain special characters that could break the JSON structure or inject malicious data.

See here for more reference: https://www.comparitech.com/net-admin/json-injection-guide/

Impact

The special character filtering was not applied to the user inputted data, resulting in tokenURI returning data that cannot be correctly parsed into JSON formats and leaving the tokenURI vulnerable to a JSON injection attack. Injected scripts can be used to perform cross-site scripting (XSS) attacks if the JSON data is rendered in a web application.

Tools Used

Manual review

Recommendations

Check for special characters also to prevent the possibility of this happening. All user-provided data should be sanitized before being included in the JSON object according to this: https://github.com/OWASP/json-sanitizer

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

SVG Injection

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.