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
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
_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.
Manual review
Changing _mint
to _safeMint
, provided the function is safe from re-entrancy
https://github.com/Cyfrin/2024-05-Sablier/issues/1
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.