Sablier

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

Impossible withdraw if the asset is USDC/USDT and the recipient address is blacklisted

Summary

The SablierV2Lockup::withdrawMultiple function reverts if one of the recipient addresses is blacklisted, particularly when using USDC or USDT tokens as the asset.

Vulnerability Details

The SablierV2Lockup::withdrawMultiple function iterates over the array of streamIds and calls withdraw from each stream to the recipient:

function withdrawMultiple(
uint256[] calldata streamIds,
uint128[] calldata amounts
)
external
override
noDelegateCall
{
// Check: there is an equal number of `streamIds` and `amounts`.
uint256 streamIdsCount = streamIds.length;
uint256 amountsCount = amounts.length;
if (streamIdsCount != amountsCount) {
revert Errors.SablierV2Lockup_WithdrawArrayCountsNotEqual(streamIdsCount, amountsCount);
}
// Iterate over the provided array of stream IDs, and withdraw from each stream to the recipient.
for (uint256 i = 0; i < streamIdsCount; ++i) {
// Checks, Effects and Interactions: check the parameters and make the withdrawal.
@> withdraw({ streamId: streamIds[i], to: _ownerOf(streamIds[i]), amount: amounts[i] });
}
}

Then the withdraw function calls the internal _withdraw function:

function withdraw(
uint256 streamId,
address to,
uint128 amount
)
public
override
noDelegateCall
notNull(streamId)
updateMetadata(streamId)
{
// Check: the stream is not depleted.
if (_streams[streamId].isDepleted) {
revert Errors.SablierV2Lockup_StreamDepleted(streamId);
}
// Check: the withdrawal address is not zero.
if (to == address(0)) {
revert Errors.SablierV2Lockup_WithdrawToZeroAddress(streamId);
}
// Check: the withdraw amount is not zero.
if (amount == 0) {
revert Errors.SablierV2Lockup_WithdrawAmountZero(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);
}
// Check: the withdraw amount is not greater than the withdrawable amount.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (amount > withdrawableAmount) {
revert Errors.SablierV2Lockup_Overdraw(streamId, amount, withdrawableAmount);
}
// Retrieve the sender from storage.
address sender = _streams[streamId].sender;
// Effects and Interactions: make the withdrawal.
@> _withdraw(streamId, to, amount);
// 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 { }
}
}

And at the end the _withdraw function calls the safeTransfer function to transfer the amount to the to address:

function _withdraw(uint256 streamId, address to, uint128 amount) internal {
// Effect: update the withdrawn amount.
_streams[streamId].amounts.withdrawn = _streams[streamId].amounts.withdrawn + amount;
// Retrieve the amounts from storage.
Lockup.Amounts memory amounts = _streams[streamId].amounts;
// Using ">=" instead of "==" for additional safety reasons. In the event of an unforeseen increase in the
// withdrawn amount, the stream will still be marked as depleted.
if (amounts.withdrawn >= amounts.deposited - amounts.refunded) {
// Effect: mark the stream as depleted.
_streams[streamId].isDepleted = true;
// Effect: make the stream not cancelable anymore, because a depleted stream cannot be canceled.
_streams[streamId].isCancelable = false;
}
// Retrieve the ERC-20 asset from storage.
IERC20 asset = _streams[streamId].asset;
// Interaction: perform the ERC-20 transfer.
@> asset.safeTransfer({ to: to, value: amount });
// Log the withdrawal.
emit ISablierV2Lockup.WithdrawFromLockupStream(streamId, to, asset, amount);
}

In the documentation is written that the protocol will work with any ERC-20 compatible token:

Sablier protocol is compatible with the following:
Any network which is EVM compatible
Any ERC20 token

It is important to know that:

Some tokens (e.g. USDC, USDT) have a contract level admin controlled address blocklist. If an address is blocked, then transfers to and from that address are forbidden.

https://github.com/d-xo/weird-erc20#tokens-with-blocklists

The problem is that USDC and USDT tokens have built-in compliance mechanisms that prevent transfers to blacklisted addresses. When attempting to transfer these assets to a blacklisted address, the transfer will fail, causing the entire withdrawMultiple transaction to revert.

Impact

If one of the recipient addresses is blacklisted and the asset used is USDC or USDT, the withdrawMultiple function will revert and nobody will be able to receive the expected amount. The token transfer to these users will fail and will also brick the withdrawn system because the blacklisted user is never cleared.
The withdrawn distribution is handled in a loop, causing the recipient unable to receive the expected amount, the transfer will not be cleared and the contract will be left DOS'd. A malicious recipient can also use this as a point of attack by intentionally getting himself blacklisted.

Also, the assets can be pausable tokens:

Some tokens can be paused by an admin (e.g. BNB, ZIL).

Similary to the blocklist issue above, an admin controlled pause feature opens users of the token to risk from a malicious or compromised
token owner.

https://github.com/d-xo/weird-erc20?tab=readme-ov-file#pausable-tokens

If the asset that is used is pausable token and is paused, then the transfer of this token from and to the protocol will be impossible.
In the considered withdrawMultiple function that means again the withdrawn functunality will be broken and the recipients will be unable to receive their expected amount.

Tools Used

Manual Review

Recommendations

To prevent the withdrawMultiple function from reverting when one of the recipient addresses is blacklisted, you can implement a pre-check mechanism to identify and skip blacklisted addresses before attempting the withdrawal.

Updates

Lead Judging Commences

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.