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

Use _safeTransfer instead of _transfer

0xaman

Summary

The Protocol allows the recipient of stream to transfer the NFT to other recipient However it does not use _safeTransfer to check either the new recipient is capable to receiving the NFT if new Recipient is smart contract.

Vulnerability Details

Let's have a look at withdrawMaxAndTransfer function which will transfer the NFT to new Recipient.

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 }); // @audit-issue : use safe Transfer
}

If the newRecipient is smart contract it will not be aware of the NFT and NFT will get lost.
Note: This issue is not listed in bot report.

Impact

Use _safeTransfer instead _transfer to make sure that if newRecipient is smart contract and compatible with NFT standard.

Tools Used

Manual Review

Recommendations

use _safeTransfer instead of _transfer

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Info/Gas/Invalid as per Docs

https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Support

FAQs

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