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

Stream : isTransferable restriction can be bypassed by directly calling the NFT transfer function in ERC721 contract

ak1

Summary

The stream NFT has the following implementation

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. -------->>> @@ audit find
/// @param amounts Struct containing the deposit, withdrawn, and refunded amounts, both denoted in units of the
/// asset's decimals.
struct Stream {

This mean, that the NFT should be transferred only when the isTransferable is set to TRUE. But this can be bypassed by directly calling the NFT transfer function from ERC721 contract.

Vulnerability Details

The SablierV2Lockup.sol is inheriting the ERC721 contract which has the NFT related functions.

Let focus on the following function.

transferFrom

safeTransferFrom

Since these functions are public, the NFT owner can use them to transfer the NFT to any address which they want.

Impact

This isTransferable check which is set by the ERC20 sender can be bypassed. It is against the current implementation.

When the ERC20 should be withdrawn by the recipient who is decided by the sender by setting the isTransferable flag.

The above function would be used to bypass them.

Tools Used

Manual review.

Recommendations

Overwrite the public NFT transfer functions from ERC721 contract and make sure that the isTransferable is set to true when transferring the NFT.

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.