Sablier
Sablier
DeFiFoundry
53,440 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`SablierV2MerkleLockup.sol` - `NAME` is using `bytes32` and decoding is incorrect because of it

EgisSecurity

Summary

When a SablierV2MerkleLockup is deployed, one of the things in the constructor that is set is NAME, which is of data type bytes32:

constructor(MerkleLockup.ConstructorParams memory params) {
// Check: the campaign name is not greater than 32 bytes
if (bytes(params.name).length > 32) {
revert Errors.SablierV2MerkleLockup_CampaignNameTooLong({
nameLength: bytes(params.name).length,
maxLength: 32
});
}
​
admin = params.initialAdmin;
ASSET = params.asset;
CANCELABLE = params.cancelable;
EXPIRATION = params.expiration;
ipfsCID = params.ipfsCID;
MERKLE_ROOT = params.merkleRoot;
NAME = bytes32(abi.encodePacked(params.name));
TRANSFERABLE = params.transferable;
}

Vulnerability Details

The name is abi.encodePackedand then casted tobytes32`.

This is a problem, as when we call name() the code is attempting to cast a bytes32 to string, this is an issue, because bytes32 is of fixed size, meaning that there will be a lot of 0's that will be incorrectly decoded.

Example:

params.name = "Test"
​
NAME = 0x5465737400000000000000000000000000000000000000000000000000000000
name() = "Test\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000"

All the \u0000 are 0's that are cast to string, which doesn't work right.

Impact

Wrong representational string format.

Tools Used

Manual Review

Recommendations

Change the data type of NAME from bytes32 to bytes, as bytes isn't fixed size and the encoding/decoding will be correct.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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