Aura Finance contest - cryptphi's results

1. Missing array length match: The function below fails to perform input validation on arrays to verify the lengths match. A mismatch could lead to an exception or undefined behavior. AuraVestedEscrow.fund()  - https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraVestedEscrow.sol#L96 Booster.voteGaugeWeight() - https://github.com/code-423n4/2022-05-aura/blob/main/convex-platform/contracts/contracts/Booster.sol#L535

2. Use of deprecated safeApprove(): The following functions use the deprecated safeApprove AuraBalRewardPool.constructor() - https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraBalRewardPool.sol#L75 AuraPenaltyForwarder.constructor - https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraPenaltyForwarder.sol#L41 AuraVestedEscrow._claim() CrvDepositorWrapper._setApprovals()

The OpenZeppelin ERC20 safeApprove() function has been deprecated, as seen in the comments of the OpenZeppelin code. Using this deprecated function can lead to unintended reverts and potentially the locking of funds. A deeper discussion on the deprecation of this function is in OZ As suggested by the OpenZeppelin comment, replace safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead.

3. Missing approve(0): CrvDepositorWrapper.setApproval() - https://github.com/code-423n4/2022-05-aura/blob/main/contracts/CrvDepositorWrapper.sol#L119.

4. ** UpdatePool should be internal:** ConvexMasterChef.updatePool() is currently public instead of internal - https://github.com/code-423n4/2022-05-aura/blob/main/convex-platform/contracts/contracts/ConvexMasterChef.sol#L186

5. Missing zero address check: Aura.constructor() AuraVestedEscrow.setAdmin() - missing zero address in constructor and so admin cannot be changed in setAdmin() and setLocker(), cancel() revert DepositToken.constructor() - operator parameter , makes contract useless PoolManagerProxy.sol - _operator parameter in setOperator(). If owner is malicious, could set operator to zero address and render shutdownPool() and addPool() useless. PoolManagerV3sol - _operator parameter in setOperator(). If the current Operator is malicious, operator could set operator to zero address and render shutdownPool() , setProtectPool() and addPool() useless. StashFactoryV2.sol - _operator parameter in constructor would require the contract to be redeployed since it would make the contract useless. TokenFactory.sol - _operator parameter in constructor would require the contract to be redeployed since it would make the contract useless. VirtualBalanceRewardPool.sol - _op param in constructor would make queueNewRewards() revert always. cCrv.sol - _operator parameter in setOperator(). If the current Operator is malicious, operator could set operator to zero address and render mint() and burn() useless.

6. Unbounded loop in function: AuraLocker.getReward() includes an unbounded for loop, when the getReward() is called in line 299 and jumps to line 304, may lead to a DoS if the array of rewardsTokens is very large and BaseRewardPool.withdraw() - withdrawal may be denied/freeze if the array of extraRewards is too large since the array can keep growing. BaseRewardPool.withdrawAndUnwrapTo() - withdrawal may be denied/freeze when the call jumps to _withdrawAndUnwrapTo if the array of extraRewards is too large since the array can keep growing.