c4-bot-3 commented 7 months ago

Lines of code

Vulnerability details

C4 issue

H-05: _getReferencePoolPriceX96() will show incorrect price for negative tick deltas in current implementation cause it doesn't round up for them

Comments

Revert interacts with a UniswapV3 pool for price calculations. Part of calculating the TWAP price of the UniswapV3 pool requires retrieving data from Uniswap and running several formulas converting cumulative ticks to a square rooted price.

In calculating the TWAP's tick, Revert assumes that the delta between the two cumulative ticks does not need to be rounded down if the delta is below 0. By not rounding down when the delta is below 0, an incorrect tick value may be used to calculate the square root price. This results in returning an incorrect price.

We can confirm the rounding down requirement by referring to Uniswap's OracleLibrary:

if (tickCumulativesDelta < 0 && (tickCumulativesDelta % secondsAgo != 0)) arithmeticMeanTick--;

Mitigation

PR #10

To resolve this, Revert applies the following check when calculating the tick for a given TWAP:

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

This fix is applied to both V3Oracle._getReferencePoolPriceX96() and Automator._getTWAPTick() where the TWAP tick is calculated. In the fix (which is identical in both code locations), the if statement checks to see if the tick cumulative is negative. If the delta is below 0, the protocol then checks to see if there is any remainder with the delta tick cumulative when divided by twapSecond. If the remainder is non-zero, then the tick should be rounded down by one.

This fix is verbatim to the Uniswap original implementation:

int56 tickCumulativesDelta = tickCumulatives[1] - tickCumulatives[0];

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

Conclusion

LGTM

c4-judge commented 7 months ago

jhsagd76 marked the issue as satisfactory