search

code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

`_safeMint()` should be used rather than `_mint()`, or NFTs may be lost #213

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L271 https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L182

Vulnerability details

Impact

_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function

If an NFTCollection creator uses a contract to automate the minting of NFTs defined by their metadata path, and mistakenly does not provide interfaces for getting back out the NFTs, the NFTs may be lost. Separately, if an NFTDropCollection mints NFTs to arbitrary addresses, and those addresses are wallets that cannot support NFTs, those NFTs may be lost as well.

Proof of Concept

There are two cases where _mint() has been used instead of _safeMint():

File: contracts/NFTCollection.sol

271:        _mint(msg.sender, tokenId);

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTCollection.sol#L271

File: contracts/NFTDropCollection.sol

182:        _mint(to, i);

https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L182

Tools Used

Code inspection

Recommended Mitigation Steps

Use _safeMint() instead of _mint()

ghost commented 2 years ago

dup of https://github.com/code-423n4/2022-08-foundation-findings/issues/267

HardlyDifficult commented 2 years ago

Dupe of https://github.com/code-423n4/2022-08-foundation-findings/issues/183