QA Report

[code423n4]

safeApprove() is deprecated

Deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance()
The contract made use of the deprecated SafeERC20.safeApprove() function. As noted in the SafeERC20 contract, the safeApprove() function was vulnerable to the same transaction reordering issue as the standard ERC20 approve() function, as detailed here[https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729]

Instances:

contracts/swappers/SwapperRouter.sol:265: IERC20(token_).safeApprove(spender_, type(uint256).max);
contracts/strategies/BkdTriHopCvx.sol:72: IERC20(underlying_).safeApprove(curveHopPool_, type(uint256).max);
contracts/strategies/BkdTriHopCvx.sol:73: IERC20(hopLp_).safeApprove(curvePool_, type(uint256).max);
contracts/strategies/BkdTriHopCvx.sol:74: IERC20(lp_).safeApprove(address(BOOSTER), type(uint256).max);
contracts/strategies/BkdTriHopCvx.sol:132: IERC20(hopLp).safeApprove(curvePool, 0);
contracts/strategies/BkdTriHopCvx.sol:133: IERC20(hopLp).safeApprove(curvePool_, type(uint256).max);
contracts/strategies/BkdTriHopCvx.sol:134: IERC20(lp_).safeApprove(address(BOOSTER), 0);
contracts/strategies/BkdTriHopCvx.sol:135: IERC20(lp).safeApprove(address(BOOSTER), type(uint256).max);
contracts/strategies/BkdEthCvx.sol:43: IERC20(lp).safeApprove(address(BOOSTER), type(uint256).max);
contracts/strategies/ConvexStrategyBase.sol:105: CRV.safeApprove(address(swapperRouter), type(uint256).max);
contracts/strategies/ConvexStrategyBase.sol:106: CVX.safeApprove(address(swapperRouter), type(uint256).max);
contracts/strategies/ConvexStrategyBase.sol:107: WETH.safeApprove(address(swapperRouter), type(uint256).max);
contracts/strategies/ConvexStrategyBase.sol:288: IERC20(token).safeApprove(swapperRouter, 0);
contracts/strategies/ConvexStrategyBase.sol:289: IERC20(token).safeApprove(swapperRouter, type(uint256).max);
contracts/RewardHandler.sol:52: IERC20(targetLpToken).safeApprove(address(bkdLocker), burnedAmount);
contracts/RewardHandler.sol:64: IERC20(token).safeApprove(spender, type(uint256).max);
contracts/CvxCrvRewardsLocker.sol:57: IERC20(CRV).safeApprove(CRV_DEPOSITOR, type(uint256).max);
contracts/CvxCrvRewardsLocker.sol:60: IERC20(CVX_CRV).safeApprove(CVX_CRV_STAKING, type(uint256).max);
contracts/CvxCrvRewardsLocker.sol:63: IERC20(CRV).safeApprove(CVX_CRV_CRV_CURVE_POOL, type(uint256).max);
contracts/CvxCrvRewardsLocker.sol:66: IERC20(CVX).safeApprove(CVX_LOCKER, type(uint256).max);
contracts/pool/LiquidityPool.sol:700: IERC20(lpToken).safeApprove(staker_, type(uint256).max);
contracts/testing/MockTopUpHandler.sol:33: IERC20(underlying).safeApprove(addr, amount);
contracts/zaps/PoolMigrationZap.sol:27: IERC20(underlying_).safeApprove(address(newPool_), type(uint256).max);
contracts/tokenomics/FeeBurner.sol:118: IERC20(token_).safeApprove(spender_, type(uint256).max);
contracts/tokenomics/AmmConvexGauge.sol:61: IERC20(ammToken).safeApprove(booster, type(uint256).max);
contracts/vault/Erc20Vault.sol:21: IERC20(underlying_).safeApprove(address(reserve), type(uint256).max);
contracts/vault/Erc20Vault.sol:22: IERC20(underlying_).safeApprove(_pool, type(uint256).max);
contracts/actions/topup/handlers/AaveHandler.sol:85: IERC20(token).safeApprove(spender, type(uint256).max);
contracts/actions/topup/handlers/CompoundHandler.sol:138: IERC20(token).safeApprove(spender, type(uint256).max);
contracts/actions/topup/TopUpAction.sol:104: IERC20(token).safeApprove(spender, type(uint256).max);
contracts/actions/topup/TopUpAction.sol:806: IERC20(depositToken).safeApprove(feeHandler, feeAmount);
contracts/actions/topup/TopUpAction.sol:867: IERC20(token).safeApprove(spender, type(uint256).max);

Reference:

OpenZeppelin/openzeppelin-contracts#2219

Remediation:

The SafeERC20.safeIncreaseAllowance() function should be used in place of SafeERC20.safeApprove() function.

[GalloDaSballo]

Valid but the function is used correctly throughout the codebase