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

check for `segments.length=0` in Helpers.sol:calculateSegmentTimestamps ,like it’s done for `checkCreateLockupDynamic`

lilizhu

Summary

In the core/src/libraries/Helpers.sol file, within the calculateSegmentTimestamps function, if the user inputs a segments length of 0, the function will revert due to an overflow.

Vulnerability Details

In the calculateSegmentTimestamps function, if the user inputs a segments length of 0, there will create a variable namedsegmentsWithTimestamps whose length is 0, and the segmentsWithTimestamps[0] will revert due to an overflow.

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/libraries/Helpers.sol#L16C4-L36C16

/// @dev Calculate the timestamps and return the segments.
function calculateSegmentTimestamps(LockupDynamic.SegmentWithDuration[] memory segments)
internal
view
returns (LockupDynamic.Segment[] memory segmentsWithTimestamps)
{
uint256 segmentCount = segments.length;
segmentsWithTimestamps = new LockupDynamic.Segment[](segmentCount);
​
// Make the block timestamp the stream's start time.
uint40 startTime = uint40(block.timestamp);
​
// It is safe to use unchecked arithmetic because {SablierV2LockupDynamic-_create} will nonetheless check the
// correctness of the calculated segment timestamps.
unchecked {
// The first segment is precomputed because it is needed in the for loop below.
segmentsWithTimestamps[0] = LockupDynamic.Segment({
amount: segments[0].amount,
exponent: segments[0].exponent,
timestamp: startTime + segments[0].duration
});

The same scenario in the checkCreateLockupDynamic function, but checkCreateLockupDynamic function validates the length of the variable segments.

Taking the creation of a LockupDynamic stream as an example, in the SablierV2LockupDynamic.sol file, the user creates a stream through the createWithDurations function. The createWithDurations function first calls the calculateSegmentTimestamps function, followed by the _create function. The initially called calculateSegmentTimestamps function does not check the length of segments, while the subsequently called _create function (through the checkCreateLockupDynamic function) does check the length of segments. This results in a delayed validation of the user's input for the segments length.

function createWithDurations(LockupDynamic.CreateWithDurations calldata params)
external
override
noDelegateCall
returns (uint256 streamId)
{
// Generate the canonical segments.
LockupDynamic.Segment[] memory segments = Helpers.calculateSegmentTimestamps(params.segments);
​
// Checks, Effects and Interactions: create the stream.
streamId = _create(
LockupDynamic.CreateWithTimestamps({
sender: params.sender,
recipient: params.recipient,
totalAmount: params.totalAmount,
asset: params.asset,
cancelable: params.cancelable,
transferable: params.transferable,
startTime: uint40(block.timestamp),
segments: segments,
broker: params.broker
})
);
}
function _create(LockupDynamic.CreateWithTimestamps memory params) internal returns (uint256 streamId) {
// Check: verify the broker fee and calculate the amounts.
Lockup.CreateAmounts memory createAmounts =
Helpers.checkAndCalculateBrokerFee(params.totalAmount, params.broker.fee, MAX_BROKER_FEE);
​
// Check: validate the user-provided parameters.
Helpers.checkCreateLockupDynamic(createAmounts.deposit, params.segments, MAX_SEGMENT_COUNT, params.startTime);

When create the lockupDynamic stream, The function call procedure looks like this：

  1. When calling the calculateSegmentTimestamps function, there is no check for segments.length within this function. If the user inputs a segments length of 0, the function will revert due to an overflow.

  2. The _create function calls the checkCreateLockupDynamic function, which checks the user's input for segments.length.

A similar problem occurs with `Helps.sol:calculateTrancheTimestamps中

Impact

This could save a lot of gas if the revert condition is met earlier.

Tools Used

Manual Review

Recommendations

in v2-core/src/libraries/Helpers.sol:calculateSegmentTimestamps, add the check for segments.length.

// Check: the segment count is not zero.
uint256 segmentCount = segments.length;
if (segmentCount == 0) {
revert Errors.SablierV2LockupDynamic_SegmentCountZero();
}
​
Updates

Lead Judging Commences

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