search

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

3 stars 2 forks source link

The GeVault#`poolMatchesOracle()` would always return `false` due to the overflow if the `sqrtPriceX96 >= 2^128`, which lead to reverting the transaction of rebalancing the GeVault #274

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L203 https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L368

Vulnerability details

Impact

The GeVault#poolMatchesOracle() would always return false due to the overflow if the sqrtPriceX96 is greater than or equal to 2^128, which lead to reverting the transaction of rebalancing the GeVault that is called via the GeVault#rebalance().

Proof of Concept

Within the GeVault#rebalance(), the GeVault#poolMatchesOracle() would be called for the validation like this: https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L203

  /// @notice Rebalance tickers
  /// @dev Provide the list of tickers from 
  function rebalance() public {
    require(poolMatchesOracle(), "GEV: Oracle Error"); /// @audit
    removeFromAllTicks();
    if (isEnabled) deployAssets();
  }

Within the GeVault#poolMatchesOracle(), the sqrtPriceX96 would be returned from uniswapPool.slot0() like this: https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L368

  /// @notice Checks that the pool price isn't manipulated
  function poolMatchesOracle() public view returns (bool matches){
    (uint160 sqrtPriceX96,,,,,,) = uniswapPool.slot0(); /// @audit

    uint decimals0 = token0.decimals();
    uint decimals1 = token1.decimals();
    uint priceX8 = 10**decimals0;
    // Overflow if dont scale down the sqrtPrice before div 2*192
    priceX8 = priceX8 * uint(sqrtPriceX96 / 2 ** 12) ** 2 * 1e8 / 2**168;  /// @audit
    priceX8 = priceX8 / 10**decimals1;
    uint oraclePrice = 1e8 * oracle.getAssetPrice(address(token0)) / oracle.getAssetPrice(address(token1));
    if (oraclePrice < priceX8 * 101 / 100 && oraclePrice > priceX8 * 99 / 100) matches = true;
  }

According to the Revert for sqrtPriceX96 >= 2^128 part in the other protocol's audit report, if the result of the sqrtPriceX96 is greater than or equal to 2^128, an overflow would occur like this:

Within the SimpleManager contract, the rebalance function includes a price calculation that triggers a revert if sqrtPriceX96 is greater than or equal to 2^128. The calculation involves squaring sqrtPriceX96, and if the result is larger than or equal to 2^256, an overflow occurs, leading to the revert.

Within the GeVault#poolMatchesOracle(), the sqrtPriceX96, which is returned from uniswapPool.slot0(), would be used as it is for the subsequent calculation.

However, the GeVault#poolMatchesOracle() would always return false due to the overflow if the sqrtPriceX96 is greater than or equal to 2^128, which lead to reverting the transaction of rebalancing the GeVault that is called via the GeVault#rebalance().

Because if the sqrtPriceX96 would 0 due to overflow, the priceX8 would also be 0. Thus, the matches below at the final line of the GeVault#poolMatchesOracle() would always be False (if the sqrtPriceX96 is greater than or equal to 2^128) and then it would be returned.

    if (oraclePrice < priceX8 * 101 / 100 && oraclePrice > priceX8 * 99 / 100) matches = true;

Since the matches, which is returned-value from the GeVault#poolMatchesOracle() above, would always be False (if the sqrtPriceX96 is greater than or equal to 2^128), the transaction would always be reverted at the line of the condition in the the GeVault#rebalance() below: 

    require(poolMatchesOracle(), "GEV: Oracle Error");

Remarks). By the way, the GeVault#poolMatchesOracle() would be called for the validation in the following functions as well. Thus, the transaction of the following functions would also be reverted if the same situation above.

Tools Used

Recommended Mitigation Steps

Consider using the uniswap oracle library methodology to square the value. This will prevent reverts due to overflows.

Assessed type

Under/Overflow

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #140

c4-judge commented 1 year ago

gzeon-c4 changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

gzeon-c4 marked the issue as satisfactory