QA Report

[code423n4]

Low

[L-01] Use Two-Step Transfer Pattern for Access Controls

Impact

Contracts implementing access control's, e.g. owner, should consider implementing a Two-Step Transfer pattern.
Otherwise it's possible that the role mistakenly transfers ownership to the wrong address, resulting in a loss of the role

Proof of Concept

AuraVestedEscrow.sol::setAdmin(), Booster.sol::setOwner(), Booster.sol::setFeeManager(), Booster.sol::setPoolManager(), Booster.sol::setVoteDelegate()

Recommendation

Consider adding a two-step approach when assigning new controllers.

Example

address owner;
address pendingOwner;

// ...

function setPendingOwner(address newPendingOwner) external {
    require(msg.sender == owner, "!owner");
    emit NewPendingOwner(newPendingOwner);
    pendingOwner = newPendingOwner;
}

function acceptOwnership() external {
    require(msg.sender == pendingOwner, "!pendingOwner");
    emit NewOwner(pendingOwner);
    owner = pendingOwner;
    pendingOwner = address(0);
}

[L-02] Do not use Deprecated Library Functions

Impact

The usage of deprecated library functions should be discouraged.

Proof of Concept

  contracts/AuraBalRewardPool.sol::75 => rewardToken.safeApprove(_auraLocker, type(uint256).max);
  contracts/AuraClaimZap.sol::98 => IERC20(crv).safeApprove(crvDepositWrapper, 0);
  contracts/AuraClaimZap.sol::99 => IERC20(crv).safeApprove(crvDepositWrapper, type(uint256).max);
  contracts/AuraClaimZap.sol::101 => IERC20(cvxCrv).safeApprove(cvxCrvRewards, 0);
  contracts/AuraClaimZap.sol::102 => IERC20(cvxCrv).safeApprove(cvxCrvRewards, type(uint256).max);
  contracts/AuraClaimZap.sol::104 => IERC20(cvx).safeApprove(locker, 0);
  contracts/AuraClaimZap.sol::105 => IERC20(cvx).safeApprove(locker, type(uint256).max);
  contracts/AuraLocker.sol::240 => IERC20(cvxCrv).safeApprove(cvxcrvStaking, 0);
  contracts/AuraLocker.sol::241 => IERC20(cvxCrv).safeApprove(cvxcrvStaking, type(uint256).max);
  contracts/AuraMerkleDrop.sol::131 => aura.safeApprove(address(auraLocker), 0);
  contracts/AuraMerkleDrop.sol::132 => aura.safeApprove(address(auraLocker), _amount);
  contracts/AuraPenaltyForwarder.sol::41 => token.safeApprove(address(distributor), type(uint256).max);
  contracts/AuraStakingProxy.sol::147 => IERC20(crv).safeApprove(crvDepositorWrapper, 0);
  contracts/AuraStakingProxy.sol::148 => IERC20(crv).safeApprove(crvDepositorWrapper, type(uint256).max);
  contracts/AuraStakingProxy.sol::150 => IERC20(cvxCrv).safeApprove(rewards, 0);
  contracts/AuraStakingProxy.sol::151 => IERC20(cvxCrv).safeApprove(rewards, type(uint256).max);
  contracts/AuraStakingProxy.sol::215 => _token.safeApprove(rewards, 0);
  contracts/AuraStakingProxy.sol::216 => _token.safeApprove(rewards, type(uint256).max);
  contracts/AuraVestedEscrow.sol::186 => rewardToken.safeApprove(address(auraLocker), claimable);
  contracts/BalLiquidityProvider.sol::59 => tkn.safeApprove(address(bVault), 0);
  contracts/BalLiquidityProvider.sol::60 => tkn.safeApprove(address(bVault), bal);
  contracts/CrvDepositorWrapper.sol::52 => IERC20(WETH).safeApprove(address(BALANCER_VAULT), type(uint256).max);
  contracts/CrvDepositorWrapper.sol::53 => IERC20(BAL).safeApprove(address(BALANCER_VAULT), type(uint256).max);
  convesx-platform/BaseRewardPool4626.sol::40 => IERC20(asset).safeApprove(operator_, type(uint256).max);
  convesx-platform/Booster.sol::422 => IERC20(token).safeApprove(rewardContract,0);
  convesx-platform/Booster.sol::423 => IERC20(token).safeApprove(rewardContract,_amount);
  convesx-platform/CrvDepositor.sol::199 => IERC20(minter).safeApprove(_stakeAddress,0);
  convesx-platform/CrvDepositor.sol::200 => IERC20(minter).safeApprove(_stakeAddress,_amount);
  convesx-platform/VoterProxy.sol::176 => IERC20(_token).safeApprove(_gauge, 0);
  convesx-platform/VoterProxy.sol::177 => IERC20(_token).safeApprove(_gauge, balance);
  convesx-platform/VoterProxy.sol::193 => _asset.safeApprove(rewardDeposit, 0);
  convesx-platform/VoterProxy.sol::194 => _asset.safeApprove(rewardDeposit, balance);
  convesx-platform/VoterProxy.sol::244 => IERC20(crvBpt).safeApprove(escrow, 0);
  convesx-platform/VoterProxy.sol::245 => IERC20(crvBpt).safeApprove(escrow, _value);
  convesx-platform/VoterProxy.sol::255 => IERC20(crvBpt).safeApprove(escrow, 0);
  convesx-platform/VoterProxy.sol::256 => IERC20(crvBpt).safeApprove(escrow, _value);

Recommendation

Use safeIncreaseAllowance / safeDecreaseAllowance instead of safeApprove.

Tools used

manual, slither