074.md

Deep Sepia Gazelle

High

NumaOracle::getV3SqrtPriceAvg function will return incorrect price for negative ticks due to wrong rounding

Summary

The NumaOracle::getV3SqrtPriceAvg function calculates and returns the sqrtPriceX96 price. But the function doesn't take into account the case when the difference of the ticks can be negative. In that case the function returns wrong value because of wrong rounding of the value of the tick.

Root Cause

Let's consider the NumaOracle::getV3SqrtPriceAvg function that is used to calculate the sqrtPriceX96 price:

function getV3SqrtPriceAvg(
    address _uniswapV3Pool,
    uint32 _interval
) public view returns (uint160) {
    require(_interval > 0, "interval cannot be zero");
    //Returns TWAP prices for short and long intervals
    uint32[] memory secondsAgo = new uint32[](2);
    secondsAgo[0] = _interval; // from (before)
    secondsAgo[1] = 0; // to (now)

    (int56[] memory tickCumulatives, ) = IUniswapV3Pool(_uniswapV3Pool)
        .observe(secondsAgo);

    // tick(imprecise as it's an integer) to sqrtPriceX96
    return
        TickMath.getSqrtRatioAtTick(
            int24(
@>              (tickCumulatives[1] - tickCumulatives[0]) /
                    int56(int32(_interval))
            )
        );
}

The function uses IUniswapV3Pool(_uniswapV3Pool).observe(secondsAgo) to get tickCumulatives array which is then used to calculate the tick and this tick is passed as input argument in TickMatch::getSqrtRatioAtTick function. The problem is that tickCumulatives[1] - tickCumulatives[0] can be negative. In that case the tick that is calculated as int24((tickCumulatives[1] - tickCumulatives[0]) / int56(int32(_interval))) should be rounded down as it's done in the Uniswap library.

But this is not being done in the NumaOracle::getV3SqrtPriceAvg function. And given that Solidity division truncates the result, the effect of this truncation is different depending on the sign of the difference int24(tickCumulatives[1] - tickCumulatives[0]). If the difference is positive a truncation will decrease the value of the tick, but in the case if int24(tickCumulatives[1] - tickCumulatives[0]) is negative and (tickCumulatives[1] - tickCumulatives[0]) % secondsAgo != 0, truncating its value increases the value of the tick. Then the returned tick will be bigger than it should be.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

If int24(tickCumulatives[1] - tickCumulatives[0]) is negative and (tickCumulatives[1] - tickCumulatives[0]) % secondsAgo != 0, then returned tick will be bigger then it should be, which opens possibility for some price manipulations and arbitrage opportunities.

PoC

No response

Mitigation

Calculate first the tick, check if the difference of the ticks is negative and then pass the tick as argument to the getSqrtRatioAtTick function:

function getV3SqrtPriceAvg(
    address _uniswapV3Pool,
    uint32 _interval
) public view returns (uint160) {
    require(_interval > 0, "interval cannot be zero");
    //Returns TWAP prices for short and long intervals
    uint32[] memory secondsAgo = new uint32[](2);
    secondsAgo[0] = _interval; // from (before)
    secondsAgo[1] = 0; // to (now)

    (int56[] memory tickCumulatives, ) = IUniswapV3Pool(_uniswapV3Pool)
        .observe(secondsAgo);

+   int24 tick = (tickCumulatives[1] - tickCumulatives[0]) / int56(int32(_interval));
+   if (tickCumulatives[1] - tickCumulatives[0] < 0 && (tickCumulatives[1] - tickCumulatives[0]) % secondsAgo != 0) tick--;
    
    // tick(imprecise as it's an integer) to sqrtPriceX96
-   return
-       TickMath.getSqrtRatioAtTick(
-           int24(
-              (tickCumulatives[1] - tickCumulatives[0]) /
-                    int56(int32(_interval))
-            )
-        );

+   return
+       TickMath.getSqrtRatioAtTick(tick);
}