Sablier

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