incorrect price for negative ticks due to lack of rounding down

mediumCode4rena

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/UniHelper.sol#L56-L73

Vulnerability details

Impact

The function callUniswapObserve is used to get twap price tick using IUniswapV3PoolOracle.observe.selector which is then used to calculate the int24 tick.

The problem is that in case if (tickCumulatives[1] - tickCumulatives[0]) is negative, the tick should be rounded down as it's done in the OracleLibrary from uniswap.

As result, in case if (tickCumulatives[1] - tickCumulatives[0])is negative and (tickCumulatives[1] - tickCumulatives[0]) % secondsAgo != 0, then returned tick will be bigger then it should be, hence incorrect prices would be used.

Proof of Concept

Unihelper:::callUniswapObserve():

solidity📋
        int56[] memory tickCumulatives = abi.decode(data, (int56[]));

        int24 tick = int24((tickCumulatives[1] - tickCumulatives[0]) / int56(int256(ago)));

        uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick);

In Uniswap's OracleLibrary:::consult():

solidity📋
        int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0];
        uint160 secondsPerLiquidityCumulativesDelta =
            secondsPerLiquidityCumulativeX128s[1] - secondsPerLiquidityCumulativeX128s[0];

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

Tools Used

Manual Review, Solodit, Josephdara, https://github.com/Uniswap/v3-periphery/blob/697c2474757ea89fec12a4e6db16a574fe259610/contracts/libraries/OracleLibrary.sol#L16-L41

Recommended Mitigation Steps

Round down the int24 tick:

solidity📋
if (tickCumulativesDelta < 0 && (tickCumulativesDelta % secondsAgo != 0)) tick--;

Assessed type

Math