code-423n4 / 2023-08-goodentry-findings

3 stars 2 forks source link

`GeVault.getTVL()` incorrectly assumes that decimals of aToken is 18 #111

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L396

Vulnerability details

Impact

Aave token is traded as 1:1 to underlying token. getTVL() assumes that all aTokens have 18 decimals, therefore returns 0 for low decimal tokens such as USDC, USDT. This function is used in multiple places:

  1. function latestAnswer() returns incorrect result, which is near 0
  2. In deposit it as a consequence treats token : share as 1:1
  3. The main is withdraw(). It burns liquidity, but transfers 0 tokens to user.

Proof of Concept

getTVL() must return underlying value with 1e8 precission in USD.

  /// @notice Calculate the vault total ticks value
  /// @return valueX8 Total value of the vault with 8 decimals
  function getTVL() public view returns (uint valueX8){
    for(uint k=0; k<ticks.length; k++){
      TokenisableRange t = ticks[k];
      uint bal = getTickBalance(k);
      valueX8 += bal * t.latestAnswer() / 1e18;
    }
  }

However getTickBalance() returns value with decimals equal to underlying token. If it is USDC, it will return in 1e6 because Aave token is equal to underlying token.

  /// @notice Get balance of tick deposited in GE
  /// @param index Tick index
  /// @return liquidity Amount of Ticker
  function getTickBalance(uint index) public view returns (uint liquidity) {
    TokenisableRange t = ticks[index];
    address aTokenAddress = lendingPool.getReserveData(address(t)).aTokenAddress;
    liquidity = ERC20(aTokenAddress).balanceOf(address(this));
  }

If for example 1000 USDC deposited to GeVault, getTVL() will return 1000e6 * 1e8 / 1e18 = 0

Let's have a look on withdraw(). Independently from liquidity amount, all transferred values will be 0 if getTVL() returns 0, but liquidity burned.

  function withdraw(uint liquidity, address token) public nonReentrant returns (uint amount) {
    require(poolMatchesOracle(), "GEV: Oracle Error");
    if (liquidity == 0) liquidity = balanceOf(msg.sender);
    require(liquidity <= balanceOf(msg.sender), "GEV: Insufficient Balance");
    require(liquidity > 0, "GEV: Withdraw Zero");

    uint vaultValueX8 = getTVL();
    uint valueX8 = vaultValueX8 * liquidity / totalSupply();
    amount = valueX8 * 10**ERC20(token).decimals() / oracle.getAssetPrice(token);
    uint fee = amount * getAdjustedBaseFee(token == address(token1)) / 1e4;

    _burn(msg.sender, liquidity);
    removeFromAllTicks();
    ERC20(token).safeTransfer(treasury, fee);
    uint bal = amount - fee;

    if (token == address(WETH)){
      WETH.withdraw(bal);
      payable(msg.sender).transfer(bal);
    }
    else {
      ERC20(token).safeTransfer(msg.sender, bal);
    }

    // if pool enabled, deploy assets in ticks, otherwise just let assets sit here until totally withdrawn
    if (isEnabled) deployAssets();
    emit Withdraw(msg.sender, token, amount, liquidity);
  }

Tools Used

Manual Review

Recommended Mitigation Steps

Divide by decimals of token, not 1e18 in function getTVL()

  /// @notice Calculate the vault total ticks value
  /// @return valueX8 Total value of the vault with 8 decimals
  function getTVL() public view returns (uint valueX8){
    for(uint k=0; k<ticks.length; k++){
      TokenisableRange t = ticks[k];
      uint bal = getTickBalance(k);
+     uint256 decimals = IERC20(lendingPool.getReserveData(address(t)).aTokenAddress).decimals();
-     valueX8 += bal * t.latestAnswer() / 1e18;
+     valueX8 += bal * t.latestAnswer() / 10 ** decimals;
    }
  }

Assessed type

ERC20

code423n4 commented 1 year ago

Withdrawn by T1MOH