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

SablierV2Lockup : `withdrawMaxAndTransfer` did not check whether the NFT is transferable or not

ak1

Summary

When the NFT is transferred using the function withdrawMaxAndTransfer, there was no check to ensure that whether the NFT is transferable or not.

Vulnerability Details

For each stream, there will be an NFT with some set of parameters associated with it.

For example, isTransferable - Boolean indicating if the stream NFT is transferable.

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/types/DataTypes.sol#L78-L82

/// @param isStream Boolean indicating if the struct entity exists.
/// @param isTransferable Boolean indicating if the stream NFT is transferable.
/// @param amounts Struct containing the deposit, withdrawn, and refunded amounts, both denoted in units of the
/// asset's decimals.
struct Stream {

The NFT minter will be setting this value to the NFT stream. Later the funds transferred by the sender can be withdrawn by the NFT owner.

For this, the functions in the SablierV2Lockup.sol would be used.

one of the function, withdrawMaxAndTransfer is used to transfer the funds as well as to transfer the NFT.

SablierV2Lockup.sol#L410-L433

function withdrawMaxAndTransfer(
uint256 streamId,
address newRecipient
)
external
override
noDelegateCall
notNull(streamId)
{
// Check: the caller is the current recipient. This also checks that the NFT was not burned.
address currentRecipient = _ownerOf(streamId);
if (msg.sender != currentRecipient) {
revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
}
​
// Skip the withdrawal if the withdrawable amount is zero.
uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
if (withdrawableAmount > 0) {
withdraw({ streamId: streamId, to: currentRecipient, amount: withdrawableAmount });
}
​
// Checks and Effects: transfer the NFT.
_transfer({ from: currentRecipient, to: newRecipient, tokenId: streamId });
}

But this function did not check for the isTransferable flag.

Impact

Even if the fund sender flag the NFT stream not to transfer, the NFT owner can still transfer to other account.

This breaks the one of indended functionality which is designed by Sablier.

Tools Used

Manual review.

Recommendations

Make sure that the isTransferable flag is enabled inside the withdrawMaxAndTransfer

Updates

Lead Judging Commences

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.