`twapFilter` will show incorrect price for negative ticks because it does not round up for negative ticks

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

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

Here is a similar issue which was reported in SaltyIO audit.

Proof of Concept

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

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]);
        }
    }

twapFilter function is used to get twap price tick using uniswap oracle. it uses univ3pool.observe() to get tickCumulatives array which is then used to calculate int24 tick.

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

As a result, in case if int24(tickCumulatives[i] - tickCumulatives[i+1]) is negative and (tickCumulatives[i] - tickCumulatives[i+1]) % secondsAgo != 0, then returned tick will be bigger then it should be, which opens possibility for some price manipulations and arbitrage opportunities.

Tools Used

Manual review & Past audit

Recommended Mitigation Steps

Add this line:

if (tickCumulatives[i] - tickCumulatives[i+1] < 0 && (tickCumulatives[i] - tickCumulatives[i+1]) % secondsAgo != 0) tick --;

Assessed type

Math

[c4-judge]

Picodes marked the issue as duplicate of #195

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

In the absence of real impact aside from a tiny price deviation compared to the other rounding direction, I'll downgrade to QA.