Aura Finance contest - berndartmueller's results

QA Report

Table of Contents

• Non-Critical Findings
  • [NC-01] - Outdated OpenZeppelin dependency
• Low Risk
  • [L-01] - Zero-address check is missing
  • [L-02] - Use of floating pragma
  • [L-03] - AuraMinter inflation protection time discrepancy
  • [L-04] - Change of DAO governance should use two-step process

Non-Critical Findings

[NC-01] - Outdated OpenZeppelin dependency

Description

The OpenZeppelin library dependency version is 4.4.2. Vulnerabilities exist in these older version of OpenZeppelin contracts. Although the vulnerable and outdated portions of OpenZeppelin contracts may not be used by the contracts currently, it is always best to apply bug fixes and use the latest version of any dependency.

Recommended mitigation steps

Use the latest version of OpenZeppelin contracts available

Low Risk

[L-01] - Zero-address check is missing

Description

Zero-address checks are a best-practice for input validation of critical address parameters.

Findings

ExtraRewardsDistributor.sol#L37
AuraPenaltyForwarder.sol#L35
AuraPenaltyForwarder.sol#L36
CrvDepositorWrapper.sol#L44
CrvDepositorWrapper.sol#L45
CrvDepositorWrapper.sol#L46
CrvDepositorWrapper.sol#L114
AuraMinter.sol#L21
AuraMinter.sol#L22
AuraLocker.sol#L158
AuraLocker.sol#L159
AuraLocker.sol#L160
ClaimFeesHelper.sol#L35
ClaimFeesHelper.sol#L36
ClaimFeesHelper.sol#L37
AuraMerkleDrop.sol#L62
AuraMerkleDrop.sol#L64
AuraMerkleDrop.sol#L65
AuraMerkleDrop.sol#L66
AuraMerkleDrop.sol#L79
AuraMerkleDrop.sol#L106
BalLiquidityProvider.sol#L34
BalLiquidityProvider.sol#L35
BalLiquidityProvider.sol#L38
BalLiquidityProvider.sol#L39

Recommended mitigation steps

Add zero-address check:

require(address(_auraLocker) != address(0), "Zero address");

[L-02] - Use of floating pragma

Description

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

https://swcregistry.io/docs/SWC-103

Findings

Aura.sol#L2 pragma solidity ^0.8.11;
AuraBalRewardPool.sol#L2 pragma solidity ^0.8.11;
AuraClaimZap.sol#L2 pragma solidity ^0.8.11;
AuraLocker.sol#L2 pragma solidity ^0.8.11;
AuraMath.sol#L2 pragma solidity ^0.8.11;
AuraMerkleDrop.sol#L2 pragma solidity ^0.8.11;
AuraMinter.sol#L2 pragma solidity ^0.8.11;
AuraPenaltyForwarder.sol#L2 pragma solidity ^0.8.11;
AuraStakingProxy.sol#L2 pragma solidity ^0.8.11;
AuraVestedEscrow.sol#L2 pragma solidity ^0.8.11;
BalLiquidityProvider.sol#L2 pragma solidity ^0.8.11;
ClaimFeesHelper.sol#L2 pragma solidity ^0.8.11;
ExtraRewardsDistributor.sol#L2 pragma solidity ^0.8.11;

Recommended mitigation steps

Lock the pragma version and also consider known bugs (https://github.com/ethereum/solidity/releases) for the compiler version that is chosen.

Pragma statements can be allowed to float when a contract is intended for consumption by other developers, as in the case with contracts in a library or EthPM package. Otherwise, the developer would need to manually update the pragma in order to compile locally.

[L-03] - AuraMinter inflation protection time discrepancy

Description

The inflationProtectionTime storage variable is initialized in the constructor of the AuraMinter contract and according to the NatSpec comment (L10), it should be set to 4 years.

However, it is set to 156 weeks which is equal to 3 years.

Findings

AuraMinter.sol#L23

inflationProtectionTime = block.timestamp + 156 weeks; // @audit-info 156 weeks equals 3 years

Recommended mitigation steps

Change to 208 weeks or adjust NatSpec comment.

[L-04] - Change of DAO governance should use two-step process

Description

It's a best practice to use a two-step process for changes of critical settings like setDao().

Findings

AuraMerkleDrop.sol#L77-L81

function setDao(address _newDao) external {
    require(msg.sender == dao, "!auth");
    dao = _newDao;
    emit DaoSet(_newDao);
}

Recommended mitigation steps

Consider implementing a two-step process where the owner nominates an account and the nominated account needs to call an acceptDAOOwnership() function for the transfer of DAO ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.