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

The validation of startTime in Helpers.sol:_checkSegments is too strict

lilizhu

Summary

startTime being equal to the current time should be acceptable and should not cause a revert.

Vulnerability Details

In the v2-core/src/libraries/Helpers.sol:_checkSegments function, it requires that the timestamps are ordered chronologically.

When creating a lockupDynamic type stream, the checkCreateLockupDynamic function is called to validate the segments, which eventually calls _checkSegments. According to the system's design principles, when creating a stream, startTime == block.timestamp is a valid condition. Therefore, the condition startTime >= segments[0].timestamp in the _checkSegments function is too strict.

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

/// @dev Checks that:
///
/// 1. The first timestamp is strictly greater than the start time.
/// 2. The timestamps are ordered chronologically.
/// 3. There are no duplicate timestamps.
/// 4. The deposit amount is equal to the sum of all segment amounts.
function _checkSegments(
LockupDynamic.Segment[] memory segments,
uint128 depositAmount,
uint40 startTime
)
private
view
{
// Check: the start time is strictly less than the first segment timestamp.
if (startTime >= segments[0].timestamp) {
revert Errors.SablierV2LockupDynamic_StartTimeNotLessThanFirstSegmentTimestamp(
startTime, segments[0].timestamp
);
}

Impact

It is not possible to create a lockupDynamic type stream with the current block.timestamp as the start time.

Tools Used

Recommendations

/// @dev Checks that:
///
/// 1. The first timestamp is strictly greater than the start time.
/// 2. The timestamps are ordered chronologically.
/// 3. There are no duplicate timestamps.
/// 4. The deposit amount is equal to the sum of all segment amounts.
function _checkSegments(
LockupDynamic.Segment[] memory segments,
uint128 depositAmount,
uint40 startTime
)
private
view
{
// Check: the start time is strictly less than the first segment timestamp.
- if (startTime >= segments[0].timestamp) {
+ if (startTime > segments[0].timestamp) {
revert Errors.SablierV2LockupDynamic_StartTimeNotLessThanFirstSegmentTimestamp(
startTime, segments[0].timestamp
);
}
​
Updates

Lead Judging Commences

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.