cryptphi - Possible token Total Supply mismatch in ERC1155NFTProduct

[sherlock-admin]

cryptphi

medium

Possible token Total Supply mismatch in ERC1155NFTProduct

Summary

Due to a check-effect-interact pattern issue, it is possible for the token total supply to not match the number of tokens in circulation.

Vulnerability Detail

The ERC1155NFTProduct.mintByOwner calls the _mint() function before incrementing tokenSupply[id] , in the event of a re-entrancy attack by the MINT_ROLE, the amount of tokens minted would not match the value in tokenSupply[id]. The re-entry to the function from _mint() call would pass the require check in require(!_exists(id), "NFT: token already minted"); since the value for _exists(id) would be false.

Impact

Token supply mismatch

Code Snippet

https://github.com/sherlock-audit/2022-10-nftport/blob/main/evm-minting-master/contracts/templates/ERC1155NFTProduct.sol#L270-L271

Tool used

Manual Review

Recommendation

Implement proper check-effect-interact patterns.

[Evert0x]

Same as #111, this is an admin protected function. So the admin is not incentivized to to a reentrancy.

[hyperspacebunny]

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

[rayn731]

Looks good, ReentrancyGuard is used to prevent reentrancy. Also, it properly follow check-effect-interact pattern.