Sablier

Sablier
DeFiFoundry
53,440 USDC
View results
Submission Details
Severity: medium
Invalid

Vulnerability Report: Incorrect Implementation of `hasExpired()` Function Leading to Locked Funds

Summary

A critical issue has been identified in the hasExpired() function implementation within the ISablierV2MerkleStreamer contract. This flaw potentially leads to the locking of funds in scenarios where the contract does not have an explicit expiration date set (EXPIRATION = 0). According to the official documentation provided at Sablier Docs, the clawback() function should be callable when "The campaign must either be expired or not have an expiration." However, due to the current implementation of hasExpired(), the contract behaves inconsistently with this specification.

Vulnerability Details

The function hasExpired() is defined as follows:

function hasExpired() public view override returns (bool) {
return EXPIRATION > 0 && EXPIRATION <= block.timestamp;
}

In the scenario where EXPIRATION = 0, intended to represent an absence of an expiration date, the function invariably returns False. This incorrect return value conflicts with the expected behavior where campaigns without a set expiration should be considered as expired for the purposes of enabling administrative actions like token clawbacks.

When the clawback() function is called, it includes a check:

if (_hasGracePeriodPassed() && !hasExpired()) {
revert Errors.SablierV2MerkleLockup_ClawbackNotAllowed({...});
}

Given the faulty hasExpired() logic, for campaigns without an expiration (EXPIRATION = 0), this condition will always evaluate to True after the grace period has passed, thus permanently preventing the clawback() operation and locking the funds in the contract.

Impact

The impact of this vulnerability is significant as it directly affects the liquidity and management of funds within the contract:

  • Funds Lock: Administrators are unable to retrieve unclaimed funds after the grace period in campaigns designed to have no expiration, leading to permanent locking of these assets.

Tools Used

The analysis was conducted using manual code review and verification against the Solidity documentation and the Sablier contract specifications.

Recommendations

To resolve this issue and align the contract behavior with the documented functionality, it is recommended to adjust the hasExpired() function as follows:

function hasExpired() public view override returns (bool) {
return EXPIRATION == 0 || EXPIRATION <= block.timestamp;
}

This adjustment ensures that:

  • If EXPIRATION is set to 0, the function correctly interprets this as an expired state.

  • The function continues to behave correctly for cases where EXPIRATION is set to a correct timestamp.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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