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

SablierV2NFTDescriptor.sol#generateAttributes() - The function is vulnerable to JSON injection from a malicious asset

EgisSecurity

Summary

SablierV2NFTDescriptor.sol#generateAttributes() - The function is vulnerable to JSON injection from a malicious asset symbol

Vulnerability Details

The nft descriptor is used to generate the tokenURI of a stream. It uses many different fields, including the stream asset's symbol.

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

The protocol team correctly handle large symbols, so that a malicious asset can't inject malicious scripts from it symbol.

// 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";
}

But, the symbol can still use JSON injection to inject an attribute, as the symbol isn't sanitized anywhere, which means the symbol field can contain the " character, which would alter the integrity of the JSON data that is being generated by generateAttributes.

function generateAttributes(
string memory assetSymbol,
string memory sender,
string memory status
)
internal
pure
returns (string memory)
{
return string.concat(
'[{"trait_type":"Asset","value":"',
assetSymbol,
'"},{"trait_type":"Sender","value":"',
sender,
'"},{"trait_type":"Status","value":"',
status,
'"}]'
);
}

If asset.symbol() returns something like '","name":"hello world', then this new attribute will be appended to the JSON data and will look something like this:

[
{
"trait_type": "Asset",
"value": "",
"name": "hello world" <- injected attribute
},
{
"trait_type": "Sender",
"value": "sender"
},
{
"trait_type": "Status",
"value": "status"
}
]

The <= 30 bytes requirement is enough to even inject images/gifs which may be very bad for the protocol, as a malicious actor can inject violence/pornographic images, which can lead to real world consequences for the protocol.

This attack can very easily be achieved with a malicious ERC20 token used when creating a stream, that is specifically built to inject malicious characters when tokenURI is called on SablierV2Lockup.

Anyone that queries that specific streamId or queries all the token URI's of the contract, will be hit with a maliciously injected attribute.

Some similar issues found in the past.
https://code4rena.com/reports/2023-12-revolutionprotocol#h-03-verbstokentokenuri-is-vulnerable-to-json-injection-attacks
https://code4rena.com/reports/2023-03-canto-identity#m-04-bio-protocol---tokenuri-json-injection

Impact

JSON injection

Tools Used

Manual Review

Recommendations

Sanitize the asset symbol, so it can't inject malicious attributes. This resource can help 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.