Sablier

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

`_safeMint` in place of `_mint`

Summary

Using _safeMint in place of _mint, in functions obeying the Checks-Effects-Interaction pattern, is advisable as the tokens can get locked in the receiver's contract

Vulnerability Details

In SablierV2LockupDynamic.sol, SablierV2LockupLinear.sol, SablierV2LockupTranched.sol, the below line is used for minting the NFT to the recipient -

_mint({ to: params.recipient, tokenId: streamId })

The use of _mint function is generally discouraged in contracts which have no risk of re-entrancy

Please refer - https://github.com/OpenZeppelin/openzeppelin-contracts/blob/52c36d412e8681053975396223d0ea39687fe33b/contracts/token/ERC721/ERC721.sol#L275

Impact

_safeMint function checks if the receiver is an Externally Owned Account or a smart contract, that is capable of managing NFTs. If the receiver is a smart contract and is not capable of managing NFTs, or, in other words, has not implented IERC721Receiver.onERC721Received, the tokens can become permanently stuck.

However, the catch here is that the function can be vulnerable to reentrancy attacks if the Checks-Effects-Interaction pattern is not followed in its design.

Tools Used

Manual review

Recommendations

Changing _mint to _safeMint, provided the function is safe from re-entrancy

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Known - LightChaser

https://github.com/Cyfrin/2024-05-Sablier/issues/1

Support

FAQs

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