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

Use _safeMint instead of _mint for ERC721 in _create function

lilizhu

Summary

Use _safeMint instead of _mint for ERC721

Vulnerability Details

The params.recipient will be minted as a proof of staking NFT when _create() is called.

However, if params.recipient is a contract address that does not support ERC721, the NFT can be frozen in the contract.

As per the documentation of EIP-721:

A wallet/broker/auction application MUST implement the wallet interface if it will accept safe transfers.

Ref: https://eips.ethereum.org/EIPS/eip-721

As per the documentation of ERC721.sol by Openzeppelin

Ref: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L274-L285

/**
* @dev Mints `tokenId` and transfers it to `to`.
*
* WARNING: Usage of this method is discouraged, use {_safeMint} whenever possible
*
* Requirements:
*
* - `tokenId` must not exist.
* - `to` cannot be the zero address.
*
* Emits a {Transfer} event.
*/
function _mint(address to, uint256 tokenId) internal {
if (to == address(0)) {
revert ERC721InvalidReceiver(address(0));
}
address previousOwner = _update(to, tokenId, address(0));
if (previousOwner != address(0)) {
revert ERC721InvalidSender(address(0));
}
}
​

Impact

Users possibly lose their NFTs

Code Snippet

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/SablierV2LockupLinear.sol#L274

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/SablierV2LockupDynamic.sol#L354

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/SablierV2LockupTranched.sol#L258

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

Tool used

Manual Review

Recommendation

Use _safeMint instead of _mint to check received address support for ERC721 implementation.

- _mint({ to: params.recipient, tokenId: streamId });
+ _safeMint({ to: params.recipient, tokenId: streamId });
Updates

Lead Judging Commences

inallhonesty Lead Judge over 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.