Incorrect deviation check

Summary

Deviation is calculated incorrectly allowing more than permitted manipulations of pool spot prices

Vulnerability Detail

When computing the deviations of prices from TWAP in Bunni and UniV3 Deviation.isDeviatingWithBpsCheck() always uses the highest of the two values as the denominator


    function isDeviating(
        uint256 value0_,
        uint256 value1_,
        uint256 deviationBps_,
        uint256 deviationMax_
    ) internal pure returns (bool) {
        return
            (value0_ < value1_)
                ? _isDeviating(value1_, value0_, deviationBps_, deviationMax_)
                : _isDeviating(value0_, value1_, deviationBps_, deviationMax_);
    }

    function _isDeviating(
        uint256 value0_,
        uint256 value1_,
        uint256 deviationBps_,
        uint256 deviationMax_
    ) internal pure returns (bool) {
        return ((value0_ - value1_) * deviationMax_) / value0_ > deviationBps_;
    }

    function _validateReserves(
        BunniKey memory key_,
        BunniLens lens_,
        uint16 twapMaxDeviationBps_,
        uint32 twapObservationWindow_
    ) internal view {

        .....

        if (
            // `isDeviatingWithBpsCheck()` will revert if `deviationBps` is invalid.
            Deviation.isDeviatingWithBpsCheck(
                reservesTokenRatio,
                twapTokenRatio,
                twapMaxDeviationBps_,
                TWAP_MAX_DEVIATION_BASE
            )
        ) {
            revert BunniPrice_PriceMismatch(address(key_.pool), twapTokenRatio, reservesTokenRatio);
        }

    function getTokenPrice(
        address lookupToken_,
        uint8 outputDecimals_,
        bytes calldata params_
    ) external view returns (uint256) {

        ......

        if (
            // `isDeviatingWithBpsCheck()` will revert if `deviationBps` is invalid.
            Deviation.isDeviatingWithBpsCheck(
                baseInQuotePrice,
                baseInQuoteTWAP,
                params.maxDeviationBps,
                DEVIATION_BASE
            )
        ) {
            revert UniswapV3_PriceMismatch(address(params.pool), baseInQuoteTWAP, baseInQuotePrice);
        }

This allows for underreporting of the deviation causing the pool to be manipulated outside of acceptable limits

Example

TWAP = 100 Spot Price = 200 maxDeviationBps = 51%

Actual deviation = (200 - 100) / 100 == 100% Calculated deviation = (200 - 100) / 200 == 50%

Impact

Pools can be manipulated outside of expected limits

Code Snippet

deviation lib https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/libraries/Deviation.sol#L69

bunni deviation check https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/BunniPrice.sol#L255-L265

uniV3 deviation check https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/UniswapV3Price.sol#L227-L235

Tool used

Manual Review

Recommendation

Keep TWAP in the denominator

Duplicate of #193

sherlock-audit / 2023-11-olympus-judging

hash - Incorrect deviation check #174