approve return values not checked

[code423n4]

Handle

cmichel

Vulnerability details

The strategies use approve but don't check the return value, see for example AaveYield._depositERC20:

function _depositERC20(address asset, uint256 amount) internal returns (address aToken, uint256 sharesReceived) {
  aToken = liquidityToken(asset);
  uint256 aTokensBefore = IERC20(aToken).balanceOf(address(this));

  address lendingPool = ILendingPoolAddressesProvider(lendingPoolAddressesProvider).getLendingPool();

  //approve collateral to vault
  // @audit should check ret value
  IERC20(asset).approve(lendingPool, 0);
  IERC20(asset).approve(lendingPool, amount);

  //lock collateral in vault
  AaveLendingPool(lendingPool).deposit(asset, amount, address(this), referralCode);

  sharesReceived = IERC20(aToken).balanceOf(address(this)).sub(aTokensBefore);
}

The ERC20.approve() function returns a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Impact

Tokens that don't actually perform the approval and return false are still counted as a correct transfer.

Recommended Mitigation Steps

We recommend using OpenZeppelin’s SafeERC20 versions with the safeApprove function that handles the return value check as well as non-standard-compliant tokens.

[0xean]

moving to non-critical as the deposit will fail if the approval isnt valid.