QA Report

[code423n4]

1. Can create stash with an empty gauge (low)

CreateStash can create a misconfigured stashes that have zero gauge as a result of operational mistake. This is possible as calls with zero gauge will pass, isV* calls will all return true, i.e. it's possible to create stash of any type with zero gauge.

Proof of Concept

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/StashFactoryV2.sol#L53-L55

    //Create a stash contract for the given gauge.
    //function calls are different depending on the version of curve gauges so determine which stash type is needed
    function CreateStash(uint256 _pid, address _gauge, address _staker, uint256 _stashVersion) external returns(address){

x0.call will return true:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/StashFactoryV2.sol#L87-L103

    function IsV1(address _gauge) private returns(bool){
        bytes memory data = abi.encode(rewarded_token);
        (bool success,) = _gauge.call(data);
        return success;
    }

    function IsV2(address _gauge) private returns(bool){
        bytes memory data = abi.encodeWithSelector(reward_tokens,uint256(0));
        (bool success,) = _gauge.call(data);
        return success;
    }

    function IsV3(address _gauge) private returns(bool){
        bytes memory data = abi.encodeWithSelector(rewards_receiver,address(0));
        (bool success,) = _gauge.call(data);
        return success;
    }

Recommended Mitigation Steps

Consider checking _gauge and _staker addresses to be non-zero.

2. Comment is misleading (low)

If it describes the _checkDelay == 0 && _relock case, it should be now + (1)

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L386-L389

        // e.g. now = 16
        // if contract is shutdown OR latest lock unlock time (e.g. 17) <= now - (1)
        // e.g. 17 <= (16 + 1)
        if (isShutdown || locks[length - 1].unlockTime <= expiryTime) {

Recommended Mitigation Steps

Consider expanding the comment to cover the both cases, processExpiredLocks and kickExpiredLocks:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L382-L384

        uint256 expiryTime = _checkDelay == 0 && _relock
            ? block.timestamp.add(rewardsDuration)
            : block.timestamp.sub(_checkDelay);

3. Comments misspelling (non-critical)

separate in both cases:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/Booster.sol#L411

//some gauges claim rewards when depositing, stash them in a seperate contract until next claim

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/BaseRewardPool.sol#L57

 *          distribute a child reward token (i.e. a secondary one from Curve, or a seperate one).

Responsible in both cases:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/Booster.sol#L569

*         Repsonsible for collecting the crv from gauge, and then redistributing to the correct place.

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/Booster.sol#L631

*         Repsonsible for collecting the crv from gauge, and then redistributing to the correct place.

Array:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L94

* @param _amount     Arrary of amount of rewardTokens to vest

4. Ownership is transferred with one step procedure in multiple cases (non-critical)

One step process offers no protection for the cases when ownership transfer is performed mistakenly or with any malicious intent.

Adding a modest complexity of an additional step and a delay is a low price to pay for having time to evaluate the ownership change.

Proof of Concept

Booster.owner:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/Booster.sol#L128-L133

    function setOwner(address _owner) external {
        require(msg.sender == owner, "!auth");
        owner = _owner;

        emit OwnerUpdated(_owner);
    }

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/BoosterOwner.sol#L107-L113

    function setBoosterOwner() external onlyOwner{
        //allow reverting ownership until sealed
        require(!isSealed, "ownership sealed");

        //transfer booster ownership to this owner
        IOwner(booster).setOwner(owner);
    }

PoolManagerProxy.owner:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/PoolManagerProxy.sol#L42-L45

    //set owner - only OWNER
    function setOwner(address _owner) external onlyOwner{
        owner = _owner;
    }

owner:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/PoolManagerSecondaryProxy.sol#L57-L60

    //set owner - only OWNER
    function setOwner(address _owner) external onlyOwner{
        owner = _owner;
    }

owner:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/VoterProxy.sol#L73-L76

    function setOwner(address _owner) external {
        require(msg.sender == owner, "!auth");
        owner = _owner;
    }

admin:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L73-L80

    /**
     * @notice Change contract admin
     * @param _admin New admin address
     */
    function setAdmin(address _admin) external {
        require(msg.sender == admin, "!auth");
        admin = _admin;
    }

DAO change:

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/AuraMerkleDrop.sol#L79

Recommended Mitigation Steps

Consider utilizing two-step ownership transferring process (proposition and acceptance in the separate actions) with a noticeable delay between the steps to enforce the transparency and stability of the system.

5. Arrays lengths aren't checked (non-critical)

AuraVestedEscrow.fund:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L96-L100

    function fund(address[] calldata _recipient, uint256[] calldata _amount) external nonReentrant {
        require(!initialised, "initialised already");

        uint256 totalAmount = 0;
        for (uint256 i = 0; i < _recipient.length; i++) {

ArbitartorVault.distribute:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/convex-platform/contracts/contracts/ArbitartorVault.sol#L42-L49

    /**
    * @notice  Permissioned fn to distribute any accrued rewards to a relevant stash
    * @dev     Only called by operator: ConvexMultisig
    */
    function distribute(address _token, uint256[] calldata _toPids, uint256[] calldata _amounts) external {
       require(msg.sender == operator, "!auth");

       for(uint256 i = 0; i < _toPids.length; i++){

6. Floating pragma is used across the system (non-critical)

As different compiler versions have critical behavior specifics if the contracts get accidentally deployed using another compiler version compared to the one they were tested with, various types of undesired behavior can be introduced.

Proof of Concept

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraVestedEscrow.sol#L2

pragma solidity ^0.8.11;

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/Aura.sol#L2

pragma solidity ^0.8.11;

Recommended Mitigation Steps

Consider fixing the version:

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/Interfaces.sol#L2

7. Scaling multiplier hardcode can lead to future mistakes (non-critical)

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/ExtraRewardsDistributor.sol#L97-L98

        uint256 rPerT = (_amount * 1e20) / supply;
        rewardData[_token][_epoch] += rPerT;

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/ExtraRewardsDistributor.sol#L257

return (balance * rewardData[_token][_epoch]) / 1e20;

8. Config addresses are not checked (non-critical)

AuraBalRewardPool:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraBalRewardPool.sol#L62-L75

    constructor(
        address _stakingToken,
        address _rewardToken,
        address _rewardManager,
        address _auraLocker,
        address _penaltyForwarder,
        uint256 _startDelay
    ) {
        stakingToken = IERC20(_stakingToken);
        rewardToken = IERC20(_rewardToken);
        rewardManager = _rewardManager;
        auraLocker = IAuraLocker(_auraLocker);
        penaltyForwarder = _penaltyForwarder;
        rewardToken.safeApprove(_auraLocker, type(uint256).max);

AuraStakingProxy:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraStakingProxy.sol#L58-L80

    /**
     * @param _rewards       vlCVX
     * @param _crv           CRV token
     * @param _cvx           CVX token
     * @param _cvxCrv        cvxCRV token
     * @param _crvDepositorWrapper    Wrapper that converts CRV to CRVBPT and deposits
     * @param _outputBps     Configurable output bps where 100% == 10000
     */
    constructor(
        address _rewards,
        address _crv,
        address _cvx,
        address _cvxCrv,
        address _crvDepositorWrapper,
        uint256 _outputBps
    ) {
        rewards = _rewards;
        owner = msg.sender;
        crv = _crv;
        cvx = _cvx;
        cvxCrv = _cvxCrv;
        crvDepositorWrapper = _crvDepositorWrapper;
        outputBps = _outputBps;

ClaimFeesHelper:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/ClaimFeesHelper.sol#L25-L38

    /**
     * @param _booster      Booster.sol, e.g. 0xF403C135812408BFbE8713b5A23a04b3D48AAE31
     * @param _voterProxy   CVX VoterProxy e.g. 0x989AEb4d175e16225E39E87d0D97A3360524AD80
     * @param _feeDistro    FeeDistro e.g. 0xA464e6DCda8AC41e03616F95f4BC98a13b8922Dc
     */
    constructor(
        address _booster,
        address _voterProxy,
        address _feeDistro
    ) {
        booster = IBooster(_booster);
        voterProxy = _voterProxy;
        feeDistro = IFeeDistributor(_feeDistro);
    }

AuraLocker:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraLocker.sol#L147-L164

    constructor(
        string memory _nameArg,
        string memory _symbolArg,
        address _stakingToken,
        address _cvxCrv,
        address _cvxCrvStaking
    ) Ownable() {
        _name = _nameArg;
        _symbol = _symbolArg;
        _decimals = 18;

        stakingToken = IERC20(_stakingToken);
        cvxCrv = _cvxCrv;
        cvxcrvStaking = _cvxCrvStaking;

        uint256 currentEpoch = block.timestamp.div(rewardsDuration).mul(rewardsDuration);
        epochs.push(Epoch({ supply: 0, date: uint32(currentEpoch) }));
    }

AuraBalRewardPool:

https://github.com/code-423n4/2022-05-aura/blob/4989a2077546a5394e3650bf3c224669a0f7e690/contracts/AuraBalRewardPool.sol#L62-L79

    constructor(
        address _stakingToken,
        address _rewardToken,
        address _rewardManager,
        address _auraLocker,
        address _penaltyForwarder,
        uint256 _startDelay
    ) {
        stakingToken = IERC20(_stakingToken);
        rewardToken = IERC20(_rewardToken);
        rewardManager = _rewardManager;
        auraLocker = IAuraLocker(_auraLocker);
        penaltyForwarder = _penaltyForwarder;
        rewardToken.safeApprove(_auraLocker, type(uint256).max);

        require(_startDelay < 2 weeks, "!delay");
        startTime = block.timestamp + _startDelay;
    }

Recommended Mitigation Steps

Consider using zero address checks to ensure correct configuration

9. ClaimFeesHelper's claimFees can end up wasting gas if the balance condition cannot be currently met (non-critical)

Proof of Concept

If the condition can be satisfied, the loop isn't needed. If it can't the loop achieves nothing, just end up using all the gas:

https://github.com/code-423n4/2022-05-aura/blob/main/contracts/ClaimFeesHelper.sol#L50-53

Recommended Mitigation Steps

Consider removing the loop, it looks like debug code to be cleaned up.

[0xMaharishi]

code-423n4/2022-05-aura#6