Panoptic - Bauchibred's results

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/PanopticPool.sol#L852-L896

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/PanopticPool.sol#L852-L896

    function _validateSolvency(
        address user,
        TokenId[] calldata positionIdList,
        uint256 buffer
    ) internal view returns (uint256 medianData) {
        // check that the provided positionIdList matches the positions in memory
        _validatePositionList(user, positionIdList, 0);

        IUniswapV3Pool _univ3pool = s_univ3pool;
        (
            ,
            int24 currentTick,
            uint16 observationIndex,
            uint16 observationCardinality,
            ,
            ,

        ) = _univ3pool.slot0();
        int24 fastOracleTick = PanopticMath.computeMedianObservedPrice(
            _univ3pool,
            observationIndex,
            observationCardinality,
            FAST_ORACLE_CARDINALITY,
            FAST_ORACLE_PERIOD
        );

        int24 slowOracleTick;
        if (SLOW_ORACLE_UNISWAP_MODE) {
            slowOracleTick = PanopticMath.computeMedianObservedPrice(
                _univ3pool,
                observationIndex,
                observationCardinality,
                SLOW_ORACLE_CARDINALITY,
                SLOW_ORACLE_PERIOD
            );
        } else {
            (slowOracleTick, medianData) = PanopticMath.computeInternalMedian(
                observationIndex,
                observationCardinality,
                MEDIAN_PERIOD,
                s_miniMedian,
                _univ3pool
            );
        }

We can see that to get the ticks, the PanopticMath.computeMedianObservedPrice() is queried.

However the PanopticMath.computeMedianObservedPrice() expects the cardinality to be odd to get the right data, see https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/libraries/PanopticMath.sol#L160-L193

    function computeMedianObservedPrice(
        IUniswapV3Pool univ3pool,
        uint256 observationIndex,
        uint256 observationCardinality,
        uint256 cardinality,
        uint256 period
    ) external view returns (int24) {
        //(snip)
        //@audit
            // get the median of the `ticks` array (assuming `cardinality` is odd)
            return int24(Math.sort(ticks)[cardinality / 2]);
        }
    }

Evidently, this function expects the cardinality to be odd, which has also been clearly documented here https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/libraries/PanopticMath.sol#L157

/// @param cardinality The number of `periods` to in the median price array, should be odd

Where as this was ensured previously, the current scope does not do this, this is because as can be seen by the snippet below the SLOW_ORACLE_CARDINALITY has been changed from 7 in the previous scope, to 8 in the current scope.

    /// @notice Amount of Uniswap observations to include in the "slow" oracle price (in Uniswap mode).
-    uint256 internal constant SLOW_ORACLE_CARDINALITY = 7;
+    uint256 internal constant SLOW_ORACLE_CARDINALITY = 8;

This then makes the attempt to get the ticks via to return the wrong tick data since Math.sort() is being queried on false pretence, (assumption that it's odd whereas it's even).

Note that from Math.sol's implementation of sort() & quicksort(), we can see how the cardinality is expected to be odd from which the cardinality / 2 from int24(Math.sort(ticks)[cardinality / 2]) in computeMedianObservedPrice() would return the right median price.

Impact

Pricing integration are done in the wrong pretence which not only goes against the docs but also means that the wrong tick data is used to validate the solvency of a user via validateSolvency() during SLOW_ORACLE_UNISWAP_MODE , this is because the internal median being calculated via PanopticMath.computeMedianObservedPrice() is also going to be inaccurate, considering the Math.sort() getting called expects an odd cardinality, but instead it's being given an even one.

Recommended Mitigation Steps

If the intention is to increase the cardinality of the slow oracle mode, then consider increasing it to another odd value, say 9 rather than 8, i.e apply these fixes:

    /// @notice Amount of Uniswap observations to include in the "slow" oracle price (in Uniswap mode).
-    uint256 internal constant SLOW_ORACLE_CARDINALITY = 8;
+    uint256 internal constant SLOW_ORACLE_CARDINALITY = 9;

Assessed type

Context