sherlock-audit / 2022-09-knox-judging

0 stars 0 forks source link

ignacio - _SAFEMINT() SHOULD BE USED RATHER THAN _MINT() WHEREVER POSSIBLE #125

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ignacio

medium

_SAFEMINT() SHOULD BE USED RATHER THAN _MINT() WHEREVER POSSIBLE

Summary

_SAFEMINT() SHOULD BE USED RATHER THAN _MINT() WHEREVER POSSIBLE _mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver.

When calling the following _mint or mintCountTo function for minting an NFT of a NFT collection or NFT drop collection, the OpenZeppelin’s ERC721Upgradeable contract’s _mint function is used to mint the NFT to a receiver. If such receiver is a contract that does not support the ERC721 protocol, the NFT will be locked and cannot be retrieved.

https://docs.openzeppelin.com/contracts/4.x/api/token/erc721#ERC721-_mint-address-uint256-

Vulnerability Detail

Proof of Concept The following steps can occur when minting an NFT of a NFT collection or NFT drop collection.

The _mint or mintCountTo function is called with msg.sender or the to input corresponding to a contract.

The OpenZeppelin’s ERC721Upgradeable contract’s _mint function is called with msg.sender or to used in Step 1 as the receiver address.

Since calling the OpenZeppelin’s ERC721Upgradeable contract’s _mint function does not execute the same contract’s _checkOnERC721Received function, it is unknown if the receiving contract inherits from the IERC721ReceiverUpgradeable interface and implements the onERC721Received function or not. It is possible that the receiving contract does not support the ERC721 protocol, which causes the minted NFT to be locked.

Code Snippet

/knox-contracts/contracts/queue/QueueInternal.sol#L95 /knox-contracts/contracts/vault/VaultBase.sol#L56

Tool used

Manual Review

Recommendation