Platform: Code4rena
Start Date: 11/08/2022
Pot Size: $40,000 USDC
Total HM: 8
Participants: 108
Period: 4 days
Judge: hickuphh3
Total Solo HM: 2
Id: 152
League: ETH
Rank: 75/108
Findings: 1
Award: $41.21
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Saw-mon_and_Natalie
Also found by: 0x1f8b, 0x52, 0xDjango, 0xNazgul, 0xSmartContract, 0xSolus, 0xackermann, 0xmatt, 0xsolstars, Aymen0909, Bnke0x0, Chom, Deivitto, DevABDee, Dravee, ElKu, IllIllI, JC, Kumpa, Lambda, LeoS, MiloTruck, PwnedNoMore, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, TomJ, Treasure-Seeker, Vexjon, Waze, Yiko, __141345__, apostle0x01, auditor0517, berndartmueller, bin2chen, bobirichman, brgltd, bulej93, c3phas, carlitox477, cccz, cryptphi, csanuragjain, d3e4, danb, delfin454000, durianSausage, erictee, fatherOfBlocks, gogo, iamwhitelights, joestakey, jonatascm, ladboy233, mics, oyc_109, rbserver, ret2basic, robee, rokinot, rvierdiiev, shenwilly, sikorico, simon135, thank_you, wagmi, yash90, zeesaw, zkhorse
41.2059 USDC - $41.21
The following QA issues were found during the code audit:
Total 76 instances of 5 issues.
Don't use Solidity keyword such as storage as variable name:
2022-08-foundation/contracts/FETH.sol::514 => function _freeFromEscrow(address account) private returns (AccountInfo storage) {
ERC20 operations can be unsafe due to different implementations and vulnerabilities in the standard. It is therefore recommended to always either use OpenZeppelin's SafeERC20 library or at least to wrap each operation in a require statement.
2022-08-foundation/contracts/PercentSplitETH.sol::236 => try erc20Contract.transfer(share.recipient, amountToSend) { 2022-08-foundation/contracts/PercentSplitETH.sol::245 => try erc20Contract.transfer(_shares[0].recipient, amountToSend) { 2022-08-foundation/contracts/mocks/WETH9.sol::31 => payable(msg.sender).transfer(wad);
Avoid floating pragmas for non-library contracts. While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations. A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain. It is recommended to pin to a concrete compiler version.
2022-08-foundation/contracts/FETH.sol::42 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/FoundationTreasury.sol::48 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/NFTCollection.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/NFTCollectionFactory.sol::42 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/NFTDropCollection.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/NFTDropMarket.sol::42 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/PercentSplitETH.sol::42 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IAdminRole.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/ICollectionFactory.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IERC20Approve.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IERC20IncreaseAllowance.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IFethMarket.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IGetFees.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IGetRoyalties.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/INFTCollectionInitializer.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/INFTDropCollectionInitializer.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/INFTDropCollectionMint.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IOperatorRole.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IOwnable.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IProxyCall.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IRoles.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/IRoyaltyInfo.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/interfaces/ITokenCreator.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/libraries/AddressLibrary.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/libraries/ArrayLibrary.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/libraries/BytesLibrary.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/libraries/LockedBalance.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/collections/CollectionRoyalties.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/collections/SequentialMintCollection.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/nftDropMarket/NFTDropMarketCore.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/nftDropMarket/NFTDropMarketFixedPriceSale.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/roles/AdminRole.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/roles/MinterRole.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/Constants.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/ContractFactory.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/FETHNode.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/FoundationTreasuryNode.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/Gap10000.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/MarketFees.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/MarketSharedCore.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/OZERC165Checker.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/shared/SendValueWithFallbackWithdraw.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/treasury/AdminRole.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/treasury/CollateralManagement.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/treasury/OZAccessControlUpgradeable.sol::10 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mixins/treasury/OperatorRole.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/BasicERC721.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/BasicERC721WithAccessControlMock.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/BasicERC721WithoutOwnerOfRevert.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/EmptyMockContract.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/FETHMarketMock.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/MockNFT.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/MockTreasury.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/NFTDropCollectionUnknownCreatorMock.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/NFTDropCollectionWithoutAccessControl.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/NonReceivableMock.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/ReceivableMock.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/RoyaltyRegistry/MockRoyaltyRegistry.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/WETH9.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/contracts/mocks/collections/SequentialMintCollectionMock.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/test/foundry/BytesLibrary.t.sol::3 => pragma solidity ^0.8.12; 2022-08-foundation/test/foundry/FixedPriceDrop.sol::3 => pragma solidity ^0.8.12;
The usage of deprecated library functions should be discouraged. This issue is mostly related to OpenZeppelin libraries.
2022-08-foundation/contracts/mixins/treasury/AdminRole.sol::16 => _setupRole(DEFAULT_ADMIN_ROLE, admin); 2022-08-foundation/contracts/mixins/treasury/OZAccessControlUpgradeable.sol::173 => function _setupRole(bytes32 role, address account) internal { 2022-08-foundation/contracts/mocks/BasicERC721WithAccessControlMock.sol::10 => _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
Providing NatSpec is a good practice for further development / debugging etc.
File: 2022-08-foundation/contracts/interfaces/ICollectionFactory.sol File: 2022-08-foundation/contracts/interfaces/IERC20Approve.sol File: 2022-08-foundation/contracts/interfaces/IERC20ApproveAllowance.sol File: 2022-08-foundation/contracts/interfaces/INFTCollectionInitializer.sol File: 2022-08-foundation/contracts/interfaces/INFTDropCollectionInitializer.sol File: 2022-08-foundation/contracts/interfaces/IOwnable.sol File: 2022-08-foundation/contracts/interfaces/IProxyCall.sol
#0 - HardlyDifficult
2022-08-18T16:49:44Z
Avoid naming collision (1 instance)
Invalid. That is not a variable name, but part of the type being returned.
Unsafe ERC20 Operation(s) (3 instances)
Invalid. Both contracts listed here are out of scope (one is a mock contract!)
Unspecific Compiler Version Pragma (62 instances)
Disagree. We intentionally use a floating pragma in order to make integrating with contracts easier. Other contract developers are looking to interact with our contracts and they may be on a different version than we use. The pragma selected for our contracts is the minimum required in order to correctly compile and function. This way integration is easier if they lag a few versions behind, or if they use the latest but we don't bump our packages frequently enough, and when we do upgrade versions unless there was a breaking solidity change -- it should just swap in by incrementing our npm package version.
Do not use Deprecated Library Functions (3 instances)
Agree, will switch to _grantRole here. It was also inconsistent with our other role contracts which had already made this change.
File is missing NatSpec (7 instances)
Yes fair feedback, our interface documentation needs improvement.