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

`cancel` function doesn't work when the cliff is zero in the SablierV2LookupLinear contract

0xbug

Summary

In the SablierV2LookupLinear contract, the cliff can be set to 0.
In this case, the startTime should be checked in the _calculateStreamedAmount function to ensure it returns 0.
However, the absence of this check can lead to a revert in _calculateStreamedAmount() due to an overflow error (PRBMath_MulDiv_Overflow) when the startTime is greater than block.timestamp.
Consequently, any function that references _calculateStreamedAmount can also revert.

Functions affected include:

  • refundableAmount()

  • cancel()

  • _streamedAmountOf()
    _withdrawableAmountOf()
    streamedAmountOf()
    withdraw()

Vulnerability Details

https://github.com/Cyfrin/2024-05-Sablier/blob/main/v2-core/src/SablierV2LockupLinear.sol#L193

function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
// If the cliff time is in the future, return zero.
uint256 cliffTime = uint256(_cliffs[streamId]);
uint256 blockTimestamp = block.timestamp;
if (cliffTime > blockTimestamp) { // @audit need to check cliffTime & startTime
return 0;
}
...
unchecked {
// Calculate how much time has passed since the stream started, and the stream's total duration.
uint256 startTime = uint256(_streams[streamId].startTime);
UD60x18 elapsedTime = ud(blockTimestamp - startTime); // @audit elapsedTime can be set 1.157e77 if startTime > blockTimestamp
UD60x18 totalDuration = ud(endTime - startTime);
​
// Divide the elapsed time by the stream's total duration.
UD60x18 elapsedTimePercentage = elapsedTime.div(totalDuration); // @audit PRBMath_MulDiv_Overflow
...
}
function test_CreateAndCancel()
external
whenNotDelegateCalled
whenCliffDurationCalculationDoesNotOverflow
whenTotalDurationCalculationDoesNotOverflow
{
// Declare the timestamps.
uint40 blockTimestamp = getBlockTimestamp() + defaults.TOTAL_DURATION();
LockupLinear.Timestamps memory timestamps = LockupLinear.Timestamps({
start: blockTimestamp,
cliff: blockTimestamp + defaults.CLIFF_DURATION(),
end: blockTimestamp + defaults.TOTAL_DURATION()
});
​
// Create the stream.
createDefaultStreamWithTimestamps(timestamps);
​
lockupLinear.cancel(streamId);
​
timestamps = LockupLinear.Timestamps({
start: blockTimestamp,
cliff: 0, // @audit
end: blockTimestamp + defaults.TOTAL_DURATION()
});
​
// Create the stream.
createDefaultStreamWithTimestamps(timestamps);
​
vm.expectRevert();
​
lockupLinear.cancel(streamId); // @audit reverted
}

Impact

Due to this issue, the refundableAmount() and cancel() functions don't work correctly, preventing the stream sender from canceling the stream.

Tools Used

Manual

Recommendations

Add a startTime check in the _calculateStreamedAmount function.

function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
// If the cliff time is in the future, return zero.
uint256 cliffTime = uint256(_cliffs[streamId]);
uint256 blockTimestamp = block.timestamp;
- if (cliffTime > blockTimestamp) {
+ if (cliffTime > blockTimestamp || uint256(_streams[streamId].startTime) > blockTimestamp) {
return 0;
}
...
}
Updates

Lead Judging Commences

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

In LL context `_calculateStreamedAmount` reverts if start time is in the future and clif = 0

Support

FAQs

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