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

Allowing `SablierV2Lockup.withdraw` function to be called by someone, who is neither stream's recipient nor recipient's approved spender, without recipient's consent can be adverse

rbserver

Summary

When msg.sender, who is neither the stream's recipient nor the recipient's approved spender, calls the SablierV2Lockup.withdraw function with the recipient's address as the to input, the recipient can be forced to withdraw from such stream without the recipient's consent. If the recipient or sender is a contract that implements the onLockupStreamWithdrawn function, such onLockupStreamWithdrawn function can be called unexpectedly and adversely, such as causing the recipient to withdraw much earlier than intended and fail to become more vested coercively.

Vulnerability Details

When msg.sender is neither the stream's recipient nor such recipient's approved spender, calling the following SablierV2Lockup.withdraw function with such recipient's address as the to input would not revert with Errors.SablierV2Lockup_WithdrawalAddressNotRecipient because to != recipient is false. This causes the withdraw function to be called without the consent of the stream's recipient. In this case, if the stream's recipient or sender is a contract that implements the onLockupStreamWithdrawn function, the ISablierV2Recipient(recipient).onLockupStreamWithdrawn or ISablierV2Sender(sender).onLockupStreamWithdrawn function would be called, which can be unexpected and adverse to such recipient.

https://github.com/Cyfrin/2024-05-Sablier/blob/1fe69e059dcbd71728bdfaa58dad85ff199ee6dc/v2-core/src/abstracts/SablierV2Lockup.sol#L332-L402

function withdraw(
uint256 streamId,
address to,
uint128 amount
)
public
override
noDelegateCall
notNull(streamId)
updateMetadata(streamId)
{
...
​
// Retrieve the recipient from storage.
address recipient = _ownerOf(streamId);
​
// Check: if `msg.sender` is neither the stream's recipient nor an approved third party, the withdrawal address
// must be the recipient.
if (to != recipient && !_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierV2Lockup_WithdrawalAddressNotRecipient(streamId, msg.sender, to);
}
​
...
​
// Interaction: if `msg.sender` is not the recipient and the recipient is a contract, try to invoke the
// withdraw hook on it without reverting if the hook is not implemented, and also without bubbling up
// any potential revert.
if (msg.sender != recipient && recipient.code.length > 0) {
try ISablierV2Recipient(recipient).onLockupStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}
​
// Interaction: if `msg.sender` is not the sender, the sender is a contract and is different from the
// recipient, try to invoke the withdraw hook on it without reverting if the hook is not implemented, and also
// without bubbling up any potential revert.
if (msg.sender != sender && sender.code.length > 0 && sender != recipient) {
try ISablierV2Sender(sender).onLockupStreamWithdrawn({
streamId: streamId,
caller: msg.sender,
to: to,
amount: amount
}) { } catch { }
}
}

Impact

For example, for incentivizing the recipient to not withdraw early, the stream's sender represented by a contract and recipient could agree on a vesting scheme that only allows the recipient to withdraw once in which the sender's onLockupStreamWithdrawn function would call the SablierV2Lockup.cancel function to cancel the corresponding stream and refund its unvested amount to the sender immediately after the withdraw function is called for such stream. When someone, who is not the stream's recipient, calls the withdraw function with the recipient's address as the to input after just a small stream amount has become vested, the sender's onLockupStreamWithdrawn function would be called. In this situation, the recipient fails to become more vested and cannot withdraw more stream amount even though she or he does not intend to withdraw early at all.

Tools Used

Manual Review

Recommended Mitigation

https://github.com/Cyfrin/2024-05-Sablier/blob/1fe69e059dcbd71728bdfaa58dad85ff199ee6dc/v2-core/src/abstracts/SablierV2Lockup.sol#L363-L365 can be updated to the following code.

if (!_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierV2Lockup_WithdrawalAddressNotRecipient(streamId, msg.sender, to);
}
Updates

Lead Judging Commences

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