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

The overflow in the `_calculateStreamedAmount` function can lead to unexpected results.

ethersky

Summary

In the _calculateStreamedAmount function, the calculation is within an unchecked block.
When the start time is later than the current block's timestamp, an overflow occurs during the calculation.
This can lead to several vulnerabilities:

  • The sender cannot cancel the stream before the start time because the PRB math library reverts due to the overflowed, extremely large values.
    The sender should be able to cancel the stream anytime if it has not been depleted yet.

  • For specific values, the overflow can result in incorrect calculations (without triggering a revert), allowing some tokens to be withdrawn before the start time.

Vulnerability Details

We can create a linear lockup using the createWithTimestamps function in SablierV2LockupLinear.

function createWithTimestamps(LockupLinear.CreateWithTimestamps calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Checks, Effects and Interactions: create the stream.
streamId = _create(params);
}

Obviously, the start time can be later than the current block.timestamp because some senders may want to start streaming in the future.

After some time, the sender wants to cancel their stream before the start time because they found issues with their plan.
However, this cancellation will be reverted due to an overflow.

Let's explain this step by step with a specific example.
The test for this example will be provided at the end.

The current time is 1714518000, and the start time is 1714690800, which is slightly later.
The cliff time is 0, and the duration is 10,000.
The sender wants to cancel this stream, so they call the cancel function.
Here, we calculate the streamed amount so far by calling the _calculateStreamedAmount function

function _cancel(uint256 streamId) internal {
// Calculate the streamed amount.
uint128 streamedAmount = _calculateStreamedAmount(streamId);
}

In the _calculateStreamedAmount function, we don't check whether the start time is later than the current time.
Therefore, in the unchecked block, an overflow occur.
In our case, the elapsed time becomes a large value due to overflow (almost type(256).max), and the total duration is 10,000.
We call the div function in the PRB math library.

function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
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);
}
}

In the div function, we multiply by 1e18, and the revert occurs in the mulDiv function

function div(UD60x18 x, UD60x18 y) pure returns (UD60x18 result) {
result = wrap(Common.mulDiv(x.unwrap(), uUNIT, y.unwrap()));
}
function mulDiv(uint256 x, uint256 y, uint256 denominator) pure returns (uint256 result) {
assembly ("memory-safe") {
let mm := mulmod(x, y, not(0))
prod0 := mul(x, y)
prod1 := sub(sub(mm, prod0), lt(mm, prod0))
}
if (prod1 >= denominator) {
revert PRBMath_MulDiv_Overflow(x, y, denominator); // @audit, here
}
}

In our test, the revert message is as follows:

[FAIL. Reason: PRBMath_MulDiv_Overflow(115792089237316195423570985008687907853269984665640564039457584007913129467136 [1.157e77], 1000000000000000000 [1e18], 10000 [1e4])]

Please add below test to the test/integration/concrete/lockup-linear/create-with-timestamps/createWithTimestamps.t.sol

function test_Cancel_Linear_Before_Start() external {
LockupLinear.CreateWithTimestamps memory params = defaults.createWithTimestampsLL();
params.timestamps.cliff = 0;
uint256 streamId = lockupLinear.createWithTimestamps(params);
assertGt(params.timestamps.start, block.timestamp); // params.timestamps.start = 1714690800, block.timestamp = 1714518000
​
uint256 duration = params.timestamps.end - params.timestamps.start;
assertEq(duration, 10000);
​
resetPrank({ msgSender: params.sender });
lockupLinear.cancel(streamId);
}

Impact

The impact is described in the Summary section.

Tools Used

Manual review

Recommendations

function _calculateStreamedAmount(uint256 streamId) internal view override returns (uint128) {
uint256 cliffTime = uint256(_cliffs[streamId]);
uint256 blockTimestamp = block.timestamp;
if (cliffTime > blockTimestamp) {
return 0;
}
​
+ uint256 startTime = uint256(_streams[streamId].startTime);
+ if (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

ge6a Judge
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
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.