Frontrunning and Access Control Issue on initialize() functions
Summary
The initialize() functions for Factory, ERC721NFTProduct, ERC1155NFTProduct and NFTCollection contracts allows anyone to be able to initialize the contracts and become gain the Admin role including signer, mint and burn roles.
Vulnerability Detail
Factory.initialize() function in the Factory contract is not protected by any access control. This allows anyone to be able to call the initialize function before the contract owner and initialize the contract to become admin and signer. This also means that when the factory deployer attempts to call initialize() after the contract has been deployed in a previous transaction, it can be frontrunned.
Same can be said for the ERC721NFTProduct, ERC1155NFTProduct and NFTCollection contracts when the initdata input does is not actually the initialize selector. This would deploy the contracts but not initialize them, then any user would be able to initialize the contract and add themselves to admin role and mint role and mint as much NFT as they want.
Impact
Access control issue which may lead to loss of tokens and funds.
cryptphi
medium
Frontrunning and Access Control Issue on initialize() functions
Summary
The initialize() functions for Factory, ERC721NFTProduct, ERC1155NFTProduct and NFTCollection contracts allows anyone to be able to initialize the contracts and become gain the Admin role including signer, mint and burn roles.
Vulnerability Detail
Factory.initialize() function in the Factory contract is not protected by any access control. This allows anyone to be able to call the initialize function before the contract owner and initialize the contract to become admin and signer. This also means that when the factory deployer attempts to call initialize() after the contract has been deployed in a previous transaction, it can be frontrunned.
Same can be said for the ERC721NFTProduct, ERC1155NFTProduct and NFTCollection contracts when the
initdata
input does is not actually the initialize selector. This would deploy the contracts but not initialize them, then any user would be able to initialize the contract and add themselves to admin role and mint role and mint as much NFT as they want.Impact
Access control issue which may lead to loss of tokens and funds.
Code Snippet
Factory.initialize()
https://github.com/sherlock-audit/2022-10-nftport/blob/main/evm-minting-master/contracts/Factory.sol#L99-L108ERC721NFTProduct.initialize()
https://github.com/sherlock-audit/2022-10-nftport/blob/main/evm-minting-master/contracts/templates/ERC721NFTProduct.sol#L82-L100ERC1155NFTProduct.initialize()
https://github.com/sherlock-audit/2022-10-nftport/blob/main/evm-minting-master/contracts/templates/ERC1155NFTProduct.sol#L88-L107NFTCollection.initialize()
https://github.com/sherlock-audit/2022-10-nftport/blob/main/evm-minting-master/contracts/templates/NFTCollection.sol#L141-L156Tool used
Manual Review
Recommendation
Apply necessary access controls, this will also prevent frontrunning issues.