cryptphi - Re-entrancy issue in ERC721NFTProduct mintToCaller function

[sherlock-admin]

cryptphi

high

Re-entrancy issue in ERC721NFTProduct mintToCaller function

Summary

The mintToCaller function is vulnerable to re-entrancy attack due to the function making a call to ERC721Upgradeable._safeMint()

Vulnerability Detail

When the user or contract with the MINT_ROLE calls ERC721NFTProduct.mintToCaller() , they are able to re-enter the function due to the call back in _checkOnERC721Received in the call to ERC721Upgradeable._safeMint(). This would allow the Minter to re-enter mintToCaller multiple times. Although token URI will not be set for most minted tokens in the re-entrancy attack, except for the last minted token, the token URIs can still be set by the UPDATE_TOKEN_ROLE

Impact

This may lead to minting out a token by a user and possible a loss of funds in a corresponding context.

Code Snippet

mintToCaller function with no re-entrancy protecton

function mintToCaller(
        address caller,
        uint256 tokenId,
        string memory tokenURI
    ) public onlyRole(MINT_ROLE) returns (uint256) {
        _safeMint(caller, tokenId);
        _setTokenURI(tokenId, tokenURI);
        return tokenId;
    }

Tool used

Manual Review

Recommendation

1. Implement necessary check-effect-interact patterns
2. Apply necessary non-reentrancy guards and mutexes.

[hyperspacebunny]

While re-entrancy does exist, it doesn't really have an impact since MINT_ROLE is already allowed to mint an arbitrary number of tokens. I'm not sure how loss of funds comes into play.

[Evert0x]

Considering to downgrade to low severity

[hyperspacebunny]

Fixed in https://github.com/nftport/evm-minting-sherlock-fixes/pull/6

[rayn731]

LGTM. ReentrancyGuard is used.