getUniV3TWAP will return wrong price when tick is negative

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L241 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1450

Vulnerability details

Vulnerability details

In PanopticPool.sol contract, function getUniV3TWAP() is used by protocol to get average price (in last 10 minutes):

int24 twapTick = getUniV3TWAP();

function getUniV3TWAP() internal view returns (int24 twapTick) {
        twapTick = PanopticMath.twapFilter(s_univ3pool, TWAP_WINDOW);
}

In getUniV3TWAP(), function twapFilter() is used to get twap price tick using Uniswap oracle. it uses univ3pool.observe() to get tickCumulatives array which is then used to calculate the twapTick:

function twapFilter(IUniswapV3Pool univ3pool, uint32 twapWindow) external view returns (int24) {
        uint32[] memory secondsAgos = new uint32[](20);

        int256[] memory twapMeasurement = new int256[](19);

        unchecked {
            // construct the time stots
            for (uint256 i = 0; i < 20; ++i) {
                secondsAgos[i] = uint32(((i + 1) * twapWindow) / 20);
            }

            // observe the tickCumulative at the 20 pre-defined time slots
            (int56[] memory tickCumulatives, ) = univ3pool.observe(secondsAgos);

            // compute the average tick per 30s window
            for (uint256 i = 0; i < 19; ++i) {
                twapMeasurement[i] = int24(
                    (tickCumulatives[i] - tickCumulatives[i + 1]) / int56(uint56(twapWindow / 20))
                );
            }

            // sort the tick measurements
            int256[] memory sortedTicks = Math.sort(twapMeasurement);

            // Get the median value
            return int24(sortedTicks[10]);
        }
    }

The problem is that in case if (tickCumulatives[i] - tickCumulatives[i + 1]) is negative, then the tick should be rounded down as it's done in the Uniswap library:

// Always round to negative infinity
        if (tickCumulativesDelta < 0 && (tickCumulativesDelta % secondsAgo != 0)) arithmeticMeanTick--;

As result, returned tick will be bigger then it should be, which opens possibility for some price manipulations and arbitrage opportunities.

Impact

If returned tick will be bigger then it should be, then it will cause the pricing logic to be unavailable and also will on important functions like liquidate() and forceExercise(), that rely on the data returned by this function.

(, int24 currentTick, , , , , ) = s_univ3pool.slot0();

            // Enforce maximum delta between TWAP and currentTick to prevent extreme price manipulation
            if (Math.abs(currentTick - twapTick) > MAX_TWAP_DELTA_LIQUIDATION)
                revert Errors.StaleTWAP();

Tools Used

Manual review.

Recommended Mitigation Steps

Use an example taken from the Uniswap library: https://github.com/Uniswap/v3-periphery/blob/main/contracts/libraries/OracleLibrary.sol#L36

Assessed type

Uniswap

[c4-judge]

Picodes marked the issue as duplicate of #195

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)