code-423n4 / 2022-08-foundation-findings

0 stars 0 forks source link

QA Report #219

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago
  1. The INFTDropCollectionInitializer(_implementation).initialize() call in NFTCollectionFactory.adminUpdateNFTDropCollectionImplementation() should have the _baseURI, _postRevealBaseURIHash  arguments declared as constants

  2. NFTCollectionFactory.initialize() - anybody can initialize the version of the NFT Collection, versionNFTCollection

  3. NFTCollectionFactory.sol contract does not comply with inherited ICollectionFactory contractThe NFTCollectionFactory contract inherits ICollectionFactory, however, does not comply with ICollectionFactory. There is no rolesContract() function in NFTCollectionFactory.

  4. NFTDropMarketFixedPriceSale.createFixedPriceSale() is missing a zero value check for price param.Unless allowed in the documentation, a collection admin may mistakenly input 0 as the price for the FixedPriceSale and make the sale free mint. Otherwise, there should be a zero value check in the createFixedPriceSale() function

  5. NFTDropMarketFixedPriceSale contract does not implement receive() function to be able to receive ETH in the contract.

HardlyDifficult commented 2 years ago

The INFTDropCollectionInitializer(_implementation).initialize()

Fair feedback but I don't agree here. This is a special case where the values here are basically just magic numbers / values. I think inline definitions is more clear & easier to read.

Anyone can initialize

Invalid. See our comment here for context

NFTCollectionFactory.sol contract does not comply with interface

Invalid. It does, and my inheriting from the interface the compiler guarantees this is the case for us.

createFixedPriceSale() is missing a zero value check for price

Invalid. 0 is supported and this is mentioned in the comments.

NFTDropMarketFixedPriceSale contract does not implement receive()

Invalid. FETHNode implements a receiver for that use case. Other functions are payable where required.