Inconsistent rounding in the `computeMedianObservedPrice` function

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/libraries/PanopticMath.sol#L149-L151

Vulnerability details

Proof of Concept

The function computeMedianObservedPrice computes the array of historical price ticks as:

for (uint256 i = 0; i < cardinality; ++i) {
    ticks[i] =
        (tickCumulatives[i] - tickCumulatives[i + 1]) /
        int256(timestamps[i] - timestamps[i + 1]);
}

The result of division is always rounded towards zero, both when the tickCumulatives[i] - tickCumulatives[i + 1] is a positive number, and when it's a negative number. This potentially results in bias towards zero in the ticks, from which the median tick is computed.

This is fixed the Uniswap library, which always rounds the result down.

Impact

The function is called from _validateSolvency, which uses to: 1) Decide if a position is solvent at a given price 2) Decide whether to additionally use a slow oracle to additionally validate the solvency. The slow oracle price may be calculated using the same method, so is similarly exposed to the problem.

As a result: 1) The code will incorrectly or inconsistently compute the solvency status for some positions, leading to undesired liquidations or to undesired lack of liquidations. 2) The code will fail to use the slow oracle to _checkSolvencyAtTick in some cases when it should have used it. Specifically, when the fastOracleTick is computed from a time period when the price in the pool was above 1.0 (positive ticks) most of the time, and the slowOracleTicks from time period when price was below 1.0 (negative ticks), the computed delta will be lower than the actual delta, resulting in the slow oracle check incorrectly being skipped in some situations. (Same for a symmetrical situation of fast price below 1.0 and slow price above 1.0.)

This has been accepted as valid issue in e.g here and here.

Tools Used

Manual review

Recommended Mitigation Steps

Change the code to round consistently, for example:

for (uint256 i = 0; i < cardinality; ++i) {
    int256 tickCumulativesDelta = tickCumulatives[i] - tickCumulatives[i + 1];
    int256 timeDelta = int256(timestamps[i] - timestamps[i + 1]);
    ticks[i] = tickCumulativesDelta / timeDelta;
    if (tickCumulativesDelta < 0 && tickCumulativesDelta % timeDelta != 0) {
        // Always round to negative infinity
        ticks[i]--;
    }
}

Assessed type

Math

[c4-judge]

Picodes marked the issue as duplicate of #195

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-c