search

code-423n4 / 2022-04-backd-findings

6 stars 4 forks source link

The Contract Should approve(0) first #215

Closed gzeoneth closed 2 years ago

gzeoneth commented 2 years ago

Originally submitted by warden defsec in #198, duplicate of #178 related to the use of safeApprove. This is upgraded from a QA report to standalone issue because it correctly described the revert when trying to call safeApprove on non-zero allowance. QA report that only describe safeApprove as deprecated and unable to identify the revert problem are intentionally not upgraded. Duplicate of #180.

The Contract Should approve(0) first

Impact

Some tokens (like USDT L199) do not work when changing the allowance from an existing non-zero allowance value. They must first be approved by zero and then the actual allowance must be approved.

IERC20(token).safeApprove(address(operator), 0);
IERC20(token).safeApprove(address(operator), amount);

Proof of Concept

  1. Navigate to the following contracts.
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/CvxCrvRewardsLocker.sol::53 => IERC20(CRV).safeApprove(CRV_DEPOSITOR, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/CvxCrvRewardsLocker.sol::56 => IERC20(CVX_CRV).safeApprove(CVX_CRV_STAKING, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/CvxCrvRewardsLocker.sol::59 => IERC20(CRV).safeApprove(CVX_CRV_CRV_CURVE_POOL, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/CvxCrvRewardsLocker.sol::62 => IERC20(CVX).safeApprove(CVX_LOCKER, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/TopUpAction.sol::50 => IERC20(token).safeApprove(stakerVaultAddress, depositAmount);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/TopUpAction.sol::847 => IERC20(depositToken).safeApprove(feeHandler, feeAmount);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/TopUpAction.sol::908 => IERC20(token).safeApprove(spender, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/handlers/AaveHandler.sol::53 => IERC20(underlying).safeApprove(address(lendingPool), amount);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/handlers/CompoundHandler.sol::71 => IERC20(underlying).safeApprove(address(ctoken), amount);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/actions/topup/handlers/CompoundHandler.sol::120 => IERC20(underlying).safeApprove(address(ctoken), debt);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/pool/LiquidityPool.sol::721 => IERC20(lpToken_).safeApprove(staker_, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdEthCvx.sol::43 => IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::71 => IERC20(underlying_).safeApprove(curveHopPool_, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::72 => IERC20(hopLp_).safeApprove(curvePool_, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::73 => IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::129 => IERC20(hopLp).safeApprove(curvePool_, 0);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::130 => IERC20(hopLp).safeApprove(curvePool_, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::131 => IERC20(lp_).safeApprove(address(_BOOSTER), 0);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/BkdTriHopCvx.sol::132 => IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/ConvexStrategyBase.sol::107 => _CRV.safeApprove(address(_strategySwapper), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/ConvexStrategyBase.sol::108 => _CVX.safeApprove(address(_strategySwapper), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/ConvexStrategyBase.sol::109 => _WETH.safeApprove(address(_strategySwapper), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/ConvexStrategyBase.sol::279 => IERC20(token_).safeApprove(address(_strategySwapper), 0);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/ConvexStrategyBase.sol::280 => IERC20(token_).safeApprove(address(_strategySwapper), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/strategies/StrategySwapper.sol::209 => IERC20(token_).safeApprove(spender_, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/swappers/Swapper3Crv.sol::43 => IERC20(DAI).safeApprove(SUSHISWAP, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/swappers/Swapper3Crv.sol::44 => IERC20(USDC).safeApprove(SUSHISWAP, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/swappers/Swapper3Crv.sol::45 => IERC20(USDT).safeApprove(SUSHISWAP, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/swappers/Swapper3Crv.sol::47 => IERC20(DAI).safeApprove(UNISWAP, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/swappers/Swapper3Crv.sol::48 => IERC20(USDC).safeApprove(UNISWAP, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/swappers/Swapper3Crv.sol::49 => IERC20(USDT).safeApprove(UNISWAP, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/tokenomics/AmmConvexGauge.sol::62 => IERC20(ammToken).safeApprove(booster, type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/vault/Erc20Vault.sol::21 => IERC20(underlying_).safeApprove(address(reserve), type(uint256).max);
  2022-04-backd-c856714a50437cb33240a5964b63687c9876275b/backd/contracts/vault/Erc20Vault.sol::22 => IERC20(underlying_).safeApprove(_pool, type(uint256).max);
  1. When trying to re-approve an already approved token, all transactions revert and the protocol cannot be used.

Tools Used

None

Recommended Mitigation Steps

Approve with a zero amount first before setting the actual amount. Consider use safeIncreaseAllowance and safeDecreaseAllowance.

JeeberC4 commented 2 years ago

Manually created required json file