0xhacksmithh - `clubNft` will be lost if caller doesn't implement `ERC721 Receiver`(i.e if caller unable to handle incoming Erc721 Token)

[sherlock-admin]

0xhacksmithh

medium

clubNft will be lost if caller doesn't implement ERC721 Receiver(i.e if caller unable to handle incoming Erc721 Token)

Summary

Minted clubNft will lost if receiver doesn't know how to handel upcoming Nft(ERC721 Token)

Vulnerability Detail

FootiumClubMinter Contract mints Nft(ClubNft) to any address via mint function, Which calls further safeMint() of FootiumClub contract which code is as follows

    function safeMint(address to, uint256 tokenId) 
        external
        onlyRole(MINTER_ROLE) 
        nonReentrant 
        whenNotPaused
    {
        FootiumEscrow escrow = new FootiumEscrow(address(this), tokenId);
        clubToEscrow[tokenId] = escrow;

        _mint(to, tokenId); // @audit-issue nft will be lost

        emit EscrowDeployed(tokenId, escrow);
    }

Which directly call _mint() with to address and tokenId. As we known internal function _mint() doesn't have any

_checkOnERC721Received(address(0), to, tokenId, data),
"ERC721: transfer to non ERC721Receiver implementer"

check. So Thats why newly minted Nft will be lost if receiver contract doesn't implement ERC721Receiver.

But _safeMint() have that check for user contract, So must use _safeMint

Impact

Refer Vulnerability Detail

Code Snippet

https://github.com/sherlock-audit/2023-04-footium/blob/main/footium-eth-shareable/contracts/FootiumClub.sol#L65

Tool used

Manual Review

Recommendation

Use _safeMint() instaed of _mint().

    function safeMint(address to, uint256 tokenId) 
        external
        onlyRole(MINTER_ROLE) 
        nonReentrant 
        whenNotPaused
    {
        FootiumEscrow escrow = new FootiumEscrow(address(this), tokenId);
        clubToEscrow[tokenId] = escrow;

-        _mint(to, tokenId); 
+        _safeMint(to, tokenId); 

        emit EscrowDeployed(tokenId, escrow);
    }

Duplicate of #342