hash - Incorrect deviation check

[sherlock-admin2]

hash

medium

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