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

`SablierV2LockupLinear.sol#_calculateStreamedAmount()` - If `startTime` is in the future, the function will always revert

EgisSecurity

Summary

_calculateStreamedAmount is the most important function of any stream, because it's used to calculate the amount of tokens that have been streamed, up to block.timestamp.

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) {
return 0;
}
// If the end time is not in the future, return the deposited amount.
uint256 endTime = uint256(_streams[streamId].endTime);
if (blockTimestamp >= endTime) {
return _streams[streamId].amounts.deposited;
}
​
// In all other cases, calculate the amount streamed so far. Normalization to 18 decimals is not needed
// because there is no mix of amounts with different decimals.
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);
UD60x18 totalDuration = ud(endTime - startTime);
​
// Divide the elapsed time by the stream's total duration.
UD60x18 elapsedTimePercentage = elapsedTime.div(totalDuration);
​
// Cast the deposited amount to UD60x18.
UD60x18 depositedAmount = ud(_streams[streamId].amounts.deposited);
​
// Calculate the streamed amount by multiplying the elapsed time percentage by the deposited amount.
UD60x18 streamedAmount = elapsedTimePercentage.mul(depositedAmount);
​
// Although the streamed amount should never exceed the deposited amount, this condition is checked
// without asserting to avoid locking funds in case of a bug. If this situation occurs, the withdrawn
// amount is considered to be the streamed amount, and the stream is effectively frozen.
if (streamedAmount.gt(depositedAmount)) {
return _streams[streamId].amounts.withdrawn;
}
​
// Cast the streamed amount to uint128. This is safe due to the check above.
return uint128(streamedAmount.intoUint256());
}
}

In the case for SablierV2LockupLinear, the function looks like so.

One key point, is that the protocol allows creation of streams in the future, i.e, startTime is somewhere in the future.

You can see that such streams, are PENDING when _statusOf is called for them.

function _statusOf(uint256 streamId) internal view returns (Lockup.Status) {
if (_streams[streamId].isDepleted) {
return Lockup.Status.DEPLETED;
} else if (_streams[streamId].wasCanceled) {
return Lockup.Status.CANCELED;
}
​
if (block.timestamp < _streams[streamId].startTime) {
return Lockup.Status.PENDING;
}

Vulnerability Details

The issue lies in the fact, that _calculateStreamedAmount doesn't account for this case and attempts to calculate the following:

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);
UD60x18 totalDuration = ud(endTime - startTime);
​
// Divide the elapsed time by the stream's total duration.
UD60x18 elapsedTimePercentage = elapsedTime.div(totalDuration);
​
// Cast the deposited amount to UD60x18.
UD60x18 depositedAmount = ud(_streams[streamId].amounts.deposited);
​
// Calculate the streamed amount by multiplying the elapsed time percentage by the deposited amount.
UD60x18 streamedAmount = elapsedTimePercentage.mul(depositedAmount);
​
// Although the streamed amount should never exceed the deposited amount, this condition is checked
// without asserting to avoid locking funds in case of a bug. If this situation occurs, the withdrawn
// amount is considered to be the streamed amount, and the stream is effectively frozen.
if (streamedAmount.gt(depositedAmount)) {
return _streams[streamId].amounts.withdrawn;
}
​
// Cast the streamed amount to uint128. This is safe due to the check above.
return uint128(streamedAmount.intoUint256());
}

elapsedTime will attempt to ud (wrap), blockTimestamp - startTime, which will underflow and wrap around, because we are in an unchecked block.

elapsedTime becomes massive and is then passed down to the following line.

UD60x18 elapsedTimePercentage = elapsedTime.div(totalDuration);

This will revert with the following error:

PRBMath_MulDiv_Overflow

Note that sometimes, the tx reverted on this line, but it always reverted:

UD60x18 streamedAmount = elapsedTimePercentage.mul(depositedAmount);

Impact

DoS:

  • Sender cannot cancel a pending lockup

Tools Used

Manual Review

Recommendations

Add a similar check to the one for cliffTime

if (_streams[streamId].startTime > block.timestamp) {
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.