_safeMint() should be used rather than _mint() wherever possible
Impact
In NFTCollections.sol and NFTDropCollection, eventually it is called ERC721 _mint(). Calling _mint() this way does not ensure that the receiver of the NFT is able to accept them, making possible to lose them.
_safeMint() should be used with as it checks to see if a user can properly accept an NFT and reverts otherwise.
There is no check of the address provided by the mintNFT when creating the project that it implements ERC721Receiver.
Details
_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver.
Both open OpenZeppelin and solmate have versions of this function so that NFTs aren’t lost if they’re minted to contracts that cannot transfer them back out.
HardlyDifficult
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L130 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L143 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L159 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L271 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTDropCollection.sol#L182
Vulnerability details
_safeMint() should be used rather than _mint() wherever possible
Impact
In
NFTCollections.solandNFTDropCollection, eventually it is called ERC721_mint(). Calling_mint()this way does not ensure that the receiver of the NFT is able to accept them, making possible to lose them._safeMint()should be used with as it checks to see if a user can properly accept an NFT and reverts otherwise.There is no check of the address provided by the mintNFT when creating the project that it implements ERC721Receiver.
Details
_mint()is discouraged in favor of_safeMint()which ensures that the recipient is either an EOA or implements IERC721Receiver.Both open OpenZeppelin and solmate have versions of this function so that NFTs aren’t lost if they’re minted to contracts that cannot transfer them back out.
References
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/d4d8d2ed9798cc3383912a23b5e8d5cb602f7d4b/contracts/token/ERC721/ERC721.sol#L271
Github Permalinks
https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L130 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L143 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L159 https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTCollection.sol#L271
https://github.com/code-423n4/2022-08-foundation/blob/7d6392498e8f3b8cdc22beb582188ffb3ed25790/contracts/NFTDropCollection.sol#L182
Mitigation
Use
_safeMint()as suggested by OpenZeppelin or include the check before minting.