Sablier

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

SablierV2MerkleLockUp::clawback() can be front-run and DOS

Summary

Admin calls to SablierV2MerkleLockUp::clawback() can be DOS by users front-running the transaction with claim calls. The transaction can also revert due to normal claims between the transaction being submitted and executed.

Vulnerability Details

SablierV2MerkleLockup::clawback()

function clawback(address to, uint128 amount) external override onlyAdmin {
// Check: current timestamp is over the grace period and the campaign has not expired.
if (_hasGracePeriodPassed() && !hasExpired()) {
revert Errors.SablierV2MerkleLockup_ClawbackNotAllowed({
blockTimestamp: block.timestamp,
expiration: EXPIRATION,
firstClaimTime: _firstClaimTime
});
}
// Effect: transfer the tokens to the provided address.
ASSET.safeTransfer(to, amount);
// Log the clawback.
emit Clawback(admin, to, amount);
}

When the admin calls the clawback function, they provide an amount they want to withdraw/clawback from the funds they have deposited into the Airstream to be claimable by users.

If the admin wants to withdraw all of the funds within the Airstream and they are in the grace period they have this functionality. However a user can front-run the admin's clawback transaction and claim their portion of the funds. If a user does claim this amount before the Admin's clawback transaction is executed, then their transaction will revert:

ASSET.safeTransfer(to, amount);

If the admin wants to withdraw 10000 tokens whilst the contract's balance is 11000, however 2000 tokens worth of claims have occured during the time between the transaction being sent and executed, then the clawback call will revert as the balance is now only 9000, causing the Admin to have to resend the transaction again with an updated amount. They would have to keep continuing this until they are able to withdraw as much as they can, without anyone claiming to reduce the balance below the clawback amount

Impact

An admin calling clawback can have their transaction front-run and DOS if a user/users claim enough funds to cause the contract balance to fall below the amount passed by the admin. The same issue can occur due to normal contract operations if enough claims occur between the time of the transaction being submitted and executed.

Impact: admin clawback calls can revert due to normal contract operations (with normal admin inputs into the function) or due to malicious claimers claiming before the clawback. The admin's transaction can keep getting DOS by users withdrawing, this will keep pushing back the clawback if the admin is trying to clawback the total or almost total amount. This classifies as Medium risk due to denying the admin intended core functionality to a time-sensitive function (the longer clawbacks are delayed due to DOS, the more claims are made and the admin is able to clawback less funds).

Tools Used

Manual Review

Recommendations

If the amount provided by the admin is larger than the current contract balance, withdraw the current contract balance to ensure that an admin cannot have their transaction reverted by claims while the transaction is in the mempool.

function clawback(address to, uint128 amount) external override onlyAdmin {
// Check: current timestamp is over the grace period and the campaign has not expired.
if (_hasGracePeriodPassed() && !hasExpired()) {
revert Errors.SablierV2MerkleLockup_ClawbackNotAllowed({
blockTimestamp: block.timestamp,
expiration: EXPIRATION,
firstClaimTime: _firstClaimTime
});
}
+ uint256 currentBalance = ASSET.balanceOf(address(this));
+ if (amount > currentBalance) { amount = currentBalance }
// Effect: transfer the tokens to the provided address.
ASSET.safeTransfer(to, amount);
// Log the clawback.
emit Clawback(admin, to, amount);
}

This change ensures that the admin's clawback call will not revert due to users claiming before the transaction is executed. Users can still claim as usual, and the admin can withdraw as much funds as possible below or equal to the amount provided based on the current balance.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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