Wrong tick selected by GeVault.getActiveTickIndex()

[code423n4]

Lines of code

https://github.com/GoodEntry-io/ge/blob/c7c7de57902e11e66c8186d93c5bb511b53a45b8/contracts/GeVault.sol#L470

Vulnerability details

During mitigation of M-03, the function getActiveTickIndex() has been completely rewritten.

The new logic uses the following statement to identify the active ticker (that represents the Uniswap V3 liquidity pool actively traded):

      if( (baseTokenIsToken0 && amt0 == 0) || (!baseTokenIsToken0 && amt0 == 0) ) return tickIndex;

Boolean arithmetics can reduce the above condition to the simplified:

      if(amt0 == 0) return tickIndex;

that is incorrect, and will in most cases where baseTokenIsToken0 == true return the index 0 regardless of where the market stands.

Impact

GeVault will consistently fail to deploy assets to the active ticker. Since the active ticker is the one that's traded and generates fees, always depositing outside of that ticker makes the GeVault contract completely lose its intended functionality of concentrating liquidity where it produces value.

Proof of Concept

• Set up a GeVault with the following settings:
  • Token0: USDC (base token)
  • Token1: WETH
  • Range 0: 3002e10, 3250e10
  • Range 1: 2752e10, 3000e10
  • Range 2: 2502e10, 2750e10
  • Range 3: 2252e10, 2500e10
  • Range 4: 2002e10, 2250e10
  • Range 5: 1752e10, 2000e10
  • Range 6: 1502e10, 1750e10
  • Range 7: 1252e10, 1500e10
• Have market sit at 1875 (should select range 5)
• call getActiveTickIndex() -> this will return 0

Tools Used

Code review, Foundry

Recommended Mitigation Steps

The following change on getActiveTickIndex() did work on my end:

  /// @notice Return first valid tick
  function getActiveTickIndex() public view returns (uint activeTickIndex) {
    // loop on all ticks, if underlying is only base token then we are above, and tickIndex is 2 below
    for (uint tickIndex = 0; tickIndex < ticks.length; tickIndex++){
      (uint amt0, uint amt1) = ticks[tickIndex].getTokenAmountsExcludingFees(1e18);
      console.log(baseTokenIsToken0);
      console.log("Tick %d, amt0 %d, amt1 %d", tickIndex, amt0, amt1);
      // found a tick that's above price (ie its only underlying is the base token)
-      if( (baseTokenIsToken0 && amt0 == 0) || (!baseTokenIsToken0 && amt0 == 0) ) return tickIndex;
+      if( (baseTokenIsToken0 && amt0 != 0) || (!baseTokenIsToken0 && amt1 != 0) ) return tickIndex;
    }
    // all tickers are below price
    return ticks.length;
  }

Assessed type

Uniswap

[c4-judge]

gzeon-c4 marked the issue as duplicate of #43

[c4-judge]

gzeon-c4 marked the issue as satisfactory