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

Recipient that appears innocent during stream creation can prevent sender from cancelling a stream. This is a big problem in airdrop events

emmanuel

Summary

Due to not limiting gas sent to external contract when cancelling a stream, recipient will be able to escape stream cancellation.
This makes the cancel funtion not useful.

Although it was stated on the contest page that "This is intended to support complex transactions implemented in those hooks.", this report explains how severe this issue is, especially in airdrop events.

Vulnerability Details

If a stream is cancellable, sender can call cancel() at any time to cancel the stream and get a refund.

It is stated under the "known issues" section of the contest page that:
"In SablierV2Lockup, when onLockupStreamCanceled(), onLockupStreamWithdrawn() and onLockupStreamRenounced() hooks are called, there could be gas bomb attacks because we do not limit the gas transferred to the external contract. This is intended to support complex transactions implemented in those hooks."

abstract contract SablierV2Lockup{
...
function _cancel(uint256 streamId) internal{
...
if (recipient.code.length > 0) {
try
ISablierV2Recipient(recipient).onLockupStreamCanceled({ //@audit-info recipient will waste gas to grief cancelling
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
})
{} catch {}
}
}
...
}
​

From the comment above, it shows that protocol doesn't want to fix this, but let me explain how this can be a big issue:

  1. I'm sure protocol decide not to fix this based on the assumption that sender knows recipient. So senders can check if the recipient is a contract, and check their onLockupStreamCanceled implementation before starting a stream with them.
    But the problem is, recipient can appear innocent during stream creation (that is, it can be an EoA, or a contract with a harmless onLockupStreamCanceled function). Then after sender has created the stream, recipient frontruns sender's cancel() transaction by transferring the stream NFT to a malicious contract that implements a complex onLockupStreamCanceled function that wastes all the gas(e.g. infinite looping).
    This would cause the sender's cancel() call to revert due to out of gas.

  2. This is a big issue in Sablier airdropping feature.

    • Sablier allows protocols that want to organize airdrop events to call SablierV2MerkleLockupFactory#createMerkleLL function, setting the merkle root which corresponds to the list of "recipients" eligible for the airdrop.

    • As is seen below, Creating a merkle lockup contract also allows the sender(protocol organizing airdrop) to specify a cancelable parameter, which according to the natspec, "Indicates if the stream will be cancelable after claiming."

    • Now, if the protocol organizing airdrop decides to cancel the stream to all recipients, some malicious recipients will be able to stop the sender from canceling their stream, by sending the Stream NFT to a contract that implements an onLockupStreamCanceled function that wastes gas.

    • The result is that, sender was able to cancel the airdrop for some recipients, while some were able to escape cancellation.

library MerkleLockup {
/// @notice Struct encapsulating the base constructor parameters of a MerkleLockup campaign.
/// @param asset The contract address of the ERC-20 asset to be distributed.
@> /// @param cancelable Indicates if the stream will be cancelable after claiming.
/// @param expiration The expiration of the campaign, as a Unix timestamp.
/// @param initialAdmin The initial admin of the MerkleLockup campaign.
/// @param ipfsCID The content identifier for indexing the contract on IPFS.
/// @param merkleRoot The Merkle root of the claim data.
/// @param name The name of the campaign.
/// @param transferable Indicates if the stream will be transferable after claiming.
struct ConstructorParams {
IERC20 asset;
@> bool cancelable;
uint40 expiration;
address initialAdmin;
string ipfsCID;
bytes32 merkleRoot;
string name;
bool transferable;
}
}
...
contract SablierV2MerkleLockupFactory is ISablierV2MerkleLockupFactory {
function createMerkleLL(
@> MerkleLockup.ConstructorParams memory baseParams,
ISablierV2LockupLinear lockupLinear,
LockupLinear.Durations memory streamDurations,
uint256 aggregateAmount,
uint256 recipientCount
) external returns (ISablierV2MerkleLL merkleLL) {
// Deploy the MerkleLockup contract with CREATE.
merkleLL = new SablierV2MerkleLL(baseParams, lockupLinear, streamDurations);
​
// Log the creation of the MerkleLockup contract, including some metadata that is not stored on-chain.
emit CreateMerkleLL(merkleLL, baseParams, lockupLinear, streamDurations, aggregateAmount, recipientCount);
}
}

Impact

Sender will not be able to cancel stream, even when isCancelable parameter for that stream is set to true.
This allows recipients of an airdrop to escape cancellation, whereas streams of other recipients of that airdrop gets cancelled

Tools Used

Manual Review

Recommendations

Limit the gas transferred to the external contract when calling onLockupStreamCanceled.

Updates

Lead Judging Commences

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

Known - Contest Details

https://www.codehawks.com/contests/clvb9njmy00012dqjyaavpl44

Support

FAQs

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