The mintCountTo() method is protected with the onlyMinterOrAdmin modifier. The problem is that in MinterRole.sol the grantMinter() functionality is not protected and external, so anyone can make himself a minter and can call mintCountTo()
Impact
This can result in a non-whitelisted bad actor coming in and minting all of the remaining NFTs in a collection. If the collection has already been launched and there were lots of people minting, this can result in a need to launch the collection again, which will at best lose all the gas for the previous minters. It will also damage the protocol’s reputation as people won’t be as likely minting again.
Recommendation
Add access control to the grantMinted()method. You can make it so that only already set minters (like the one set in _initializeMinterRole() ) can set new minters.
Lines of code
https://github.com/code-423n4/2022-08-foundation/blob/792e00df429b0df9ee5d909a0a5a6e72bd07cf79/contracts/NFTDropCollection.sol#L171
Vulnerability details
Proof of concept
The
mintCountTo()
method is protected with theonlyMinterOrAdmin
modifier. The problem is that inMinterRole.sol
thegrantMinter()
functionality is not protected and external, so anyone can make himself a minter and can callmintCountTo()
Impact
This can result in a non-whitelisted bad actor coming in and minting all of the remaining NFTs in a collection. If the collection has already been launched and there were lots of people minting, this can result in a need to launch the collection again, which will at best lose all the gas for the previous minters. It will also damage the protocol’s reputation as people won’t be as likely minting again.
Recommendation
Add access control to the
grantMinted()
method. You can make it so that only already set minters (like the one set in_initializeMinterRole()
) can set new minters.