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

Unhappy recipient can mishandle the stream, leading to loss of funds for the stream creator

golanger85

Summary

In SablierV2Lockup, recipients can transfer their ownership of the stream to another address, which could
be blacklisted or the zero address, leading to the loss of funds for the stream creator.

If a stream creator and a recipient have a dispute over the timely ending of a stream or some other dispute, leading to the stream creator cancelling the stream, the recipient can transfer the stream to another address.

This can happen if the stream creator and recipient don't agree with the cancellation of the stream, and the recipient demonstrates this by transferring the stream to another address, i.e blacklisted or address(0).

This is possible because ownership of the stream is determined:

address recipient = _ownerOf(streamId);

Vulnerability Details

function _cancel(uint256 streamId) internal {
// Calculate the streamed amount.
uint128 streamedAmount = _calculateStreamedAmount(streamId);
​
// Retrieve the amounts from storage.
Lockup.Amounts memory amounts = _streams[streamId].amounts;
​
// Check: the stream is not settled.
if (streamedAmount >= amounts.deposited) {
revert Errors.SablierV2Lockup_StreamSettled(streamId);
}
​
// Check: the stream is cancelable.
if (!_streams[streamId].isCancelable) {
revert Errors.SablierV2Lockup_StreamNotCancelable(streamId);
}
​
// Calculate the sender's amount.
uint128 senderAmount;
unchecked {
senderAmount = amounts.deposited - streamedAmount;
}
​
// Calculate the recipient's amount.
uint128 recipientAmount = streamedAmount - amounts.withdrawn;
​
// Effect: mark the stream as canceled.
_streams[streamId].wasCanceled = true;
​
// Effect: make the stream not cancelable anymore, because a stream can only be canceled once.
_streams[streamId].isCancelable = false;
​
// Effect: if there are no assets left for the recipient to withdraw, mark the stream as depleted.
if (recipientAmount == 0) {
_streams[streamId].isDepleted = true;
}
​
// Effect: set the refunded amount.
_streams[streamId].amounts.refunded = senderAmount;
​
// Retrieve the sender and the recipient from storage.
address sender = _streams[streamId].sender;
address recipient = _ownerOf(streamId);
​
// Retrieve the ERC-20 asset from storage.
IERC20 asset = _streams[streamId].asset;
​
// Interaction: refund the sender.
asset.safeTransfer({ to: sender, value: senderAmount });
​
// Log the cancellation.
emit ISablierV2Lockup.CancelLockupStream(streamId, sender, recipient, asset, senderAmount, recipientAmount);
​
// Emits an ERC-4906 event to trigger an update of the NFT metadata.
emit MetadataUpdate({ _tokenId: streamId });
​
// Interaction: if the recipient is a contract, try to invoke the cancel hook on the recipient without
// reverting if the hook is not implemented, and without bubbling up any potential revert.
if (recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamCanceled({
streamId: streamId,
sender: sender,
senderAmount: senderAmount,
recipientAmount: recipientAmount
}) { } catch { }
}
}

The sender calls cancel function to cancel the stream.

The function begins execution and passes the initial checks.

Before the function completes, the recipient transfers the NFT to another address.This can potentially cause issues if the new recipient is a blacklisted address or if the transfer to the zero address happens.

The safeTransfer will fail if the recipient is blacklisted.

The cancel function then attempts to complete the cancellation, but the state may now be inconsistent or invalid.

Impact

The recipient can change between the initiation of the cancel function and the point at which safeTransfer is called.
If the new recipient is a blacklisted address for the ERC-20 token, the safeTransfer will fail.

Tools Used

Manual Review

Recommendations

If you first wait, where the sender in the first part checks the state and stream status before withdrawing their funds and the other individual withdraws their funds afterwards. If done this way, the sender wont lose out.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
0xnevi Judge
about 1 year ago
golanger85 Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 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.