Lines of code

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

Vulnerability details

Impact

In UniHelper library, the callUniswapObserve is used to query the average price in the past 30 minutes. The issue with the function is that it doesn't account for a case where tick delta i.e (tickCumulatives[1] - tickCumulatives[0]) is negative and as such doesn't round up for that case unlike what how it's done in Uniswap library. As result, in case if tickCumulatives[1] - tickCumulatives[0] is negative and (tickCumulatives[1] - tickCumulatives[0]) % secondsAgo != 0, tick will be bigger then it should be which affects negatively protocol pricing as the function is used extensively in the protocol.

Proof of Concept

As can be seen in the callUniswapObserve function, it uses observe selector to get tickCumulatives array which is then used to calculate int24 tick. Now when the int24(tickCumulatives[1] - tickCumulatives[0]) is negative, then the tick should be rounded down as uniswap claims, that it shoul always be rounded down to negative infinity. But the function doesn't do this which opens possibility for some price manipulations and arbitrage opportunities as the returned tick will be bigger than it should be.

    function callUniswapObserve(IUniswapV3Pool uniswapPool, uint256 ago) internal view returns (uint160, uint256) {
        uint32[] memory secondsAgos = new uint32[](2);

        secondsAgos[0] = uint32(ago);
        secondsAgos[1] = 0;

        (bool success, bytes memory data) =
            address(uniswapPool).staticcall(abi.encodeWithSelector(IUniswapV3PoolOracle.observe.selector, secondsAgos));
            ...

            (success, data) = address(uniswapPool).staticcall(
                abi.encodeWithSelector(IUniswapV3PoolOracle.observe.selector, secondsAgos)
            );
            if (!success) {
                revertBytes(data);
            }
        }

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

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

        uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick(tick);

        return (sqrtPriceX96, ago);
    }

The function is in getSqrtTWAP to get the square root of time weighted average price which is used as major price source in getSqrtIndexPrice in PositionCalculator.sol if there's no price feed set. This is used for auto hedging or auto closing of positions in the GammaTradeMarket.sol or both. It's also used in calculating min margin which is used during trades or to calculating vault status during liquidation.

Tools Used

Manual code review

Recommended Mitigation Steps

Consider rounding down like uniswap does

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

Assessed type

Uniswap

code-423n4 / 2024-05-predy-findings

`callUniswapObserve` doesn't round up for negative ticks which will lead to wrong price being returned #197