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

6 stars 4 forks source link

Deprecated safeApprove() function #214

Closed gzeoneth closed 2 years ago

gzeoneth commented 2 years ago

Originally submitted by warden Dravee in #146, 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.

Deprecated safeApprove() function

Using this deprecated function can lead to unintended reverts and potentially the locking of funds. A deeper discussion on the deprecation of this function is in OZ issue #2219 (OpenZeppelin/openzeppelin-contracts#2219). The OpenZeppelin ERC20 safeApprove() function has been deprecated, as seen in the comments of the OpenZeppelin code.

As recommended by the OpenZeppelin comment, I suggest replacing safeApprove() with safeIncreaseAllowance() or safeDecreaseAllowance() instead:

backd/contracts/CvxCrvRewardsLocker.sol:
  53:         IERC20(CRV).safeApprove(CRV_DEPOSITOR, type(uint256).max);
  56:         IERC20(CVX_CRV).safeApprove(CVX_CRV_STAKING, type(uint256).max);
  59:         IERC20(CRV).safeApprove(CVX_CRV_CRV_CURVE_POOL, type(uint256).max);
  62:         IERC20(CVX).safeApprove(CVX_LOCKER, type(uint256).max);

backd/contracts/actions/topup/TopUpAction.sol:
   50:             IERC20(token).safeApprove(stakerVaultAddress, depositAmount);
  847:         IERC20(depositToken).safeApprove(feeHandler, feeAmount);
  908:         IERC20(token).safeApprove(spender, type(uint256).max);

backd/contracts/actions/topup/handlers/AaveHandler.sol:
  53:         IERC20(underlying).safeApprove(address(lendingPool), amount);

backd/contracts/actions/topup/handlers/CompoundHandler.sol:
   71:             IERC20(underlying).safeApprove(address(ctoken), amount);
  120:             IERC20(underlying).safeApprove(address(ctoken), debt);

backd/contracts/pool/LiquidityPool.sol:
  721:         IERC20(lpToken_).safeApprove(staker_, type(uint256).max);

backd/contracts/strategies/BkdEthCvx.sol:
  43:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);

backd/contracts/strategies/BkdTriHopCvx.sol:
   71:         IERC20(underlying_).safeApprove(curveHopPool_, type(uint256).max);
   72:         IERC20(hopLp_).safeApprove(curvePool_, type(uint256).max);
   73:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);
  129:         IERC20(hopLp).safeApprove(curvePool_, 0);
  130:         IERC20(hopLp).safeApprove(curvePool_, type(uint256).max);
  131:         IERC20(lp_).safeApprove(address(_BOOSTER), 0);
  132:         IERC20(lp_).safeApprove(address(_BOOSTER), type(uint256).max);

backd/contracts/strategies/ConvexStrategyBase.sol:
  107:         _CRV.safeApprove(address(_strategySwapper), type(uint256).max);
  108:         _CVX.safeApprove(address(_strategySwapper), type(uint256).max);
  109:         _WETH.safeApprove(address(_strategySwapper), type(uint256).max);
  279:         IERC20(token_).safeApprove(address(_strategySwapper), 0);
  280:         IERC20(token_).safeApprove(address(_strategySwapper), type(uint256).max);

backd/contracts/strategies/StrategySwapper.sol:
  209:         IERC20(token_).safeApprove(spender_, type(uint256).max);

backd/contracts/vault/Erc20Vault.sol:
  21:         IERC20(underlying_).safeApprove(address(reserve), type(uint256).max);
  22:         IERC20(underlying_).safeApprove(_pool, type(uint256).max);
JeeberC4 commented 2 years ago

Manually created required json file