_safeMint() should be used rather than _mint() wherever possible

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/ArtGobblers.sol#L356 https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/ArtGobblers.sol#L389 https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/ArtGobblers.sol#L469 https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/Pages.sol#L211 https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/Pages.sol#L251

Vulnerability details

_safeMint() should be used rather than _mint() wherever possible

Impact

In ArtGobblers.sol and Pages.sol, 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 mint NFT 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-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/ArtGobblers.sol#L356 _mint(msg.sender, gobblerId);

https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/ArtGobblers.sol#L389 _mint(msg.sender, gobblerId);

https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/ArtGobblers.sol#L469 _mint(msg.sender, gobblerId);

https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/Pages.sol#L211 _mint(msg.sender, pageId);

https://github.com/code-423n4/2022-09-artgobblers/blob/fb54f92ffcb0c13e72c84cde24c138866d9988e8/src/Pages.sol#L251 for (uint256 i = 0; i < numPages; i++) _mint(community, ++lastMintedPageId);

Mitigation

Use _safeMint() as suggested by OpenZeppelin or include the check before minting.

[Shungy]

Informational. Also the mitigation potentially introduces reentrancy risk.

[GalloDaSballo]

Dup of #321

[GalloDaSballo]

L