0xMaharishi
commented
2 years ago Some reasonable things in here 👍
Open code423n4 opened 2 years ago
0xMaharishi
commented
2 years ago Some reasonable things in here 👍
Missing Zero-address Validation
Severity: Low Context:
Aura.sol#L45-L53,Aura.sol#L128-L132,AuraBalRewardPool.sol#L62-L79,AuraClaimZap.sol#L68-L83,AuraLocker.sol#L147-L164,AuraLocker.sol#L195-L202,AuraLocker.sol#L205-L212,AuraLocker.sol#L231-L236,AuraMerkleDrop.sol#L53-L71,AuraMerkleDrop.sol#L77-L81,AuraMerkleDrop.sol#L104-L108,AuraMinter.sol#L20-L24,AuraPenaltyForwarder.sol#L30-L42,AuraStakingProxy.sol#L66-L81,AuraStakingProxy.sol#L88-L102,AuraStakingProxy.sol#L137-L140,AuraStakingProxy.sol#L157-L163,ArbitratorVault.sol#L31-L35,BaseRewardPool4626.sol#L31-L41,Booster.sol#L96-L121,Booster.sol (For ALL address setters),BoosterOwner.sol#L69-L81,BoosterOwner.sol (For ALL address setters), [cCrv.sol#L38-L41](),ConvexMasterChef.sol#L78-L88,CrvDepositor.sol#L47-L60,CrvDepositor.sol#L62-L65, [DepositToken.sol#L30-L45](),ExtraRewardStashV3.sol#L58-L60,ExtraRewardStashV3.sol#L69-L76,PoolManagerProxy.sol#L23-L30,PoolManagerProxy.sol#L43-L50,PoolManagerSecondaryProxy.sol#L58-L72,PoolManagerV3.sol#L29-L38,RewardFactory.sol#L40-L43,RewardHook.sol#L32-L35,StashFactoryV2.sol#L39-L43,TokenFactory.sol#L31-L39,VirtualBalanceRewardPool.sol#L110-L117,VoterProxy.sol#L49-L65,VoterProxy.sol#L71-L80Description: Lack of zero-address validation on address parameters may lead to reverts and force contract redeployments.
Recommendation: Add explicit zero-address validation on input parameters of address type.
Lack of Event Emission For Critical Functions
Severity: Low Context:
AuraLocker.sol#L195-L202,AuraLocker.sol#L205-L212,AuraStakingProxy.sol#L88-L121,AuraStakingProxy.sol#L137-L140,AuraStakingProxy.sol#L157-L163,ArbitratorVault.sol#L37-L40,PoolManagerSecondaryProxy.sol#L58-L77,PoolManagerProxy.sol#L43-L50,VoterProxy.sol (For ALL setters)Description: Several functions update critical parameters that are missing event emission. These should be performed to ensure tracking of changes of such critical parameters.
Recommendation: Add events to functions that change critical parameters.
Missing Time locks
Severity: Low Context:
AuraLocker.sol#L195-L202,AuraLocker.sol#L225-L228,AuraLocker.sol#L231-L236,AuraMerkleDrop.sol#L77-L108,Booster.sol#L351-L386,PoolManagerProxy.sol#L57-L59,Description: None of the onlyOwner functions that change critical protocol addresses/parameters appear to have a time lock for a time-delayed change to alert: (1) users and give them a chance to engage/exit protocol if they are not agreeable to the changes (2) team in case of compromised owner(s) and given them a chance to perform incident response.
Recommendation: Add a time lock to these functions for a time-delayed change to alert users and protect against possible malicious changes by compromised owners(s).
Missing
onlyOwnerAccess ControlSeverity: Low Context:
AuraLocker.sol#L239-L241,AuraStakingProxy.sol#L146-L152Description: This function is missing an
onlyOwnerAccess Control. Since it sets approvals and shouldn't be able to be called by anyone but the owners.Recommendation: Add an
onlyOwnerAccess ControlEvent Spamming
Severity: Low Context:
Aura.sol#L82-L86,Description: The public visibility of this function allows griefing the system via event spamming.
Recommendation: Add an Access Control to said function to prevent event spamming
Max/Infinite Approvals are Dangerous
Severity: Low Context:
AuraLocker.sol#L239-L241,AuraStakingProxy.sol#L146-L152Description: Giving max/infinite approvals to contracts is dangerous because if those contracts are exploited then they can remove all the funds from the approving addresses.
Recommendation: Check allowance and approve as much as required.
Commented Out Code
Severity: Informational Context:
VirtualBalanceRewardPool.sol#L170,VirtualBalanceRewardPool.sol#L183,Description: Having commented out code is dirty and should be used or removed.
Recommendation: Delete or use the commented out code.
Missing or Incomplete NatSpec
Severity: Informational Context:
All ContractsDescription: Some functions are missing @notice/@dev NatSpec comments for the function, @param for all/some of their parameters and @return for return values. Given that NatSpec is an important part of code documentation, this affects code comprehension, auditability and usability.
Recommendation: Add in full NatSpec comments for all functions to have complete code documentation for future use.
Floating Solidity Pragma
Severity: Informational Context:
./contractsDescription: Contracts should be deployed using the same compiler version/flags with which they have been tested. Locking the pragma (for e.g. by not using
^in pragma solidity0.5.10) ensures that contracts do not accidentally get deployed using an older compiler version with unfixed bugs.Recommendation: Remove
^from in front of your pragma version.ABI Coder v2 Is Activated By Default
>=0.8.0Severity Informational Context:
AuraLocker.sol,Interfaces.solDescription: The ABI coder v2 is now activated by default. You can activate the old coder using
pragma abicoder v1, or explicitly select v2 usingpragma abicoder v2. ABI coder v2 performs additional checks on the input and supports a larger set of types than v1.Recommendation: Remove it for clarity.
Too recent of a Pragma
Severity Informational Context:
./contractsDescription: Using too recent of a pragma is risky since they are not battle tested. A rise of a bug that wasn't known on release would cause either a hack or a need to secure funds and redeploy.
Recommendation: Use a Pragma version that has been used for sometime. I would suggest
0.8.4for the decrease of risk and still has the gas optimizations implemented.Older Version Pragma
Severity: Informational Context:
./Convex-Platform/contractsDescription: Using very old versions of Solidity prevents benefits of bug fixes and newer security checks. Using the latest versions might make contracts susceptible to undiscovered compiler bugs.
Recommendation: Consider using a more recent version such as
0.8.4.Multiple Solidity Pragma
Severity: Informational Context:
All ContractsDescription: It is better to use one Solidity compiler version across all contracts instead of different versions with different bugs and security checks.
Recommendation: Ensure all pragma versions are the same one.