Incorrect use of getTokenAmounts causes getReserves / getTVL to be overestimated

[c4-submissions]

Lines of code

https://github.com/GoodEntry-io/ge/blob/c7c7de57902e11e66c8186d93c5bb511b53a45b8/contracts/GeVault.sol#L423 https://github.com/GoodEntry-io/ge/blob/c7c7de57902e11e66c8186d93c5bb511b53a45b8/contracts/TokenisableRange.sol#L353-L354

Vulnerability details

Impact

Previously, getTVL counted the balance of TokenisableRange and multiplied it by the LP value. After reconstruction, getTVL obtains the balance of token0 and token1 through getTokenAmounts and multiplies the token value. The problem is that when calculating the balance of tokens through getTokenAmounts, there is an extra fee part, which causes the balance and TVL to be overestimated. This results in:

1. The getAdjustedBaseFee depends on getReserves, resulting in inaccurate baseFee calculations
2. Users who deposit and withdraw before can get additional funds, who deposit later will get less tokens and have to suffer losses.

Proof of Concept

  function getTokenAmounts(uint amount) public view returns (uint token0Amount, uint token1Amount){
    (token0Amount, token1Amount) = getTokenAmountsExcludingFees(amount);
    token0Amount += fee0 * amount / totalSupply();
    token1Amount += fee1 * amount / totalSupply();
  }

  function getReserves() public view returns (uint amount0, uint amount1, uint valueX8){
    amount0 = token0.balanceOf(address(this));
    amount1 = token1.balanceOf(address(this));
    for (uint k = 0; k < ticks.length; k++){
      TokenisableRange t = ticks[k];
      address aTick = lendingPool.getReserveData(address(t)).aTokenAddress;
      uint bal = ERC20(aTick).balanceOf(address(this));
      (uint amt0, uint amt1) = t.getTokenAmounts(bal);
      amount0 += amt0;
      amount1 += amt1;
    }

    valueX8 = amount0 * oracle.getAssetPrice(address(token0)) / 10**token0.decimals() 
            + amount1 * oracle.getAssetPrice(address(token1)) / 10**token1.decimals();
  }

  function getAdjustedBaseFee(bool increaseToken0) public view returns (uint adjustedBaseFeeX4) {
    uint baseFeeX4_ = uint(baseFeeX4);
    (uint res0, uint res1, ) = getReserves();
    uint value0 = res0 * oracle.getAssetPrice(address(token0)) / 10**token0.decimals();
    uint value1 = res1 * oracle.getAssetPrice(address(token1)) / 10**token1.decimals();

    if (increaseToken0)
      adjustedBaseFeeX4 = baseFeeX4_ * value0 / (value1 + 1);
    else
      adjustedBaseFeeX4 = baseFeeX4_ * value1 / (value0 + 1);

    // Adjust from -50% to +50%
    if (adjustedBaseFeeX4 < baseFeeX4_ / 2) adjustedBaseFeeX4 = baseFeeX4_ / 2;
    if (adjustedBaseFeeX4 > baseFeeX4_ * 3 / 2) adjustedBaseFeeX4 = baseFeeX4_ * 3 / 2;
  }

The impact on getAdjustedBaseFee is clear, res0 calculates more fee0 * amount / totalSupply(), res1 calculates more fee1 * amount / totalSupply(), which will affect the result of baseFee. We mainly look at the second point:

1. When the first user deposits, LP is 0, so there is no problem. LP will be minted after depositing. At this time, TVL calculation includes fee, which is overvalued.
2. The second user deposits, TVL is overvalued, so depositing tokens of the same value can only mint less than the first amount of tokens.
3. As the second user deposits, the number of LP is further pushed up, the fee is also higher, and TVL is further overvalued. At this time, the first user to withdraw money will receive additional funds.

Additional

      (uint u0, uint u1) = getTokenAmounts(missingLiq);
      uint val0 = u0 * ORACLE.getAssetPrice(address(TOKEN0.token)) / 10**TOKEN0.decimals;
      uint val1 = u1 * ORACLE.getAssetPrice(address(TOKEN1.token)) / 10**TOKEN1.decimals;
      // missing liquidity has no value, or underlying amount is 1 or less (meaning some 1 unit rounding error on low decimal tokens)
      if (missingLiqValue == 0 || (u0 <= 1 && u1 <= 1)){
        lpAmt = expectedAmount;
      }

In TokenisableRange.depositExactly, getTokenAmounts is also used instead of getTokenAmountsExcludingFees. I am not sure whether it is intentionally designed and needs to be reviewed by the sponsor.

Tools Used

Manual review

Recommended Mitigation Steps

Use getTokenAmountsExcludingFees instead of getTokenAmounts

Assessed type

Context

[c4-judge]

gzeon-c4 marked the issue as primary issue

[c4-judge]

gzeon-c4 marked the issue as duplicate of #57

[c4-judge]

gzeon-c4 marked the issue as satisfactory