Sablier

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

Discouraging users from utilizing the protocol as intended due to an incorrect `updateMetadata` modifier

Summary

Users are discouraged from utilizing the protocol as they intended due to an incorrect updateMetadata modifier.

Vulnerability Details

The hooks onLockupStreamCanceled(), onLockupStreamWithdrawn(), and onLockupStreamRenounced() in SablierV2Lockup lack parameters that limit their gas usage. According to the documentation, these hooks are intended to support complex transactions. This means that hooks designed for their intended purpose will inherently consume large amounts of gas to handle complex transactions, even without a malicious actor exploiting a gas bomb attack to inflate the gas usage in these hooks.

From the audit (see Finding 3.2.1 in the audit report https://github.com/sablier-labs/audits/blob/main/v2-core/cantina-2023-12-21.pdf), it is noted that the updateMetadata modifier has been removed from the cancel() function, but it remains in the renounce() and withdraw() functions.

In the case of renounce() function, in the worst-case scenario, 91,172 gas (1433x64) must be available at the start of the try-catch block, resulting in approximately $3.5 at current ETH/gas prices. Although the 63/64th part of the gas is rarely consumed, on average, users and recipients will likely avoid using hooks for complex transactions to minimize gas costs.

For the withdraw() function, gas usage is higher because there are two try-catch blocks—one for recipients and one for the sender. In the worst-case scenario, 5,869,568 gas (1433x64x64) must be available at the start of the first try-catch block, resulting in a large cost of $20 at current ETH/gas prices.

Impact

Even without considering the worst-case scenario, if the cost is between $0.1 and $1, users will likely be unwilling to interact with the SablierV2Lockup contract, as the cost would not be justified.

Recipients will be less inclined to withdraw their tokens as frequently as they would have without this issue, forcing them to withdraw their tokens at longer intervals. Since the withdraw function includes a onLockupStreamWithdrawn hook whose functionality is determined by the sender, recipients would also be reluctant to create complex transactions in the hooks to reduce transaction costs, even if the sender had no malicious intent.

Third-party apps and integrations will also be hesitant to withdraw tokens for recipients with complex functionality in the hooks.

This will ultimately reduce the use of hooks as intended and force users to use the renounce and withdraw functions at longer intervals than they intended.

Tools Used

Manual Review

Recommendations

Change the updateMetadata modifier to emit the event before executing the function, rather than after.
Add these lines in SablierV2Lockup.sol file :

modifier updateMetadata(uint256 streamId) {
-- _;
emit MetadataUpdate({ _tokenId: streamId });
++ _;
}
Updates

Lead Judging Commences

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

Withdraw DoS enabled by external call and `updateMetadata` modifier

0x0bserver Submitter
about 1 year ago
0xnevi Judge
about 1 year ago
0x0bserver Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
vesla0x1 Auditor
about 1 year ago
vesla0x1 Auditor
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Withdraw DoS enabled by external call and `updateMetadata` modifier

Support

FAQs

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