Pool manipulation check in BunniHelper is flawed as uncollected fees is used

Summary

Incorrect deviation check due to inclusion of uncollected fees in BunniHelper

Vulnerability Detail

_validateReserves function is used to validate that the current price have not been manipulated before calculating the price/supply of BunniTokens. It does this by checking the deviation of reservesRatio from TWAP.

    function _validateReserves(
        BunniKey memory key_,
        BunniLens lens_,
        uint16 twapMaxDeviationBps_,
        uint32 twapObservationWindow_
    ) internal view {
        uint256 reservesTokenRatio = BunniHelper.getReservesRatio(key_, lens_);
        uint256 twapTokenRatio = UniswapV3OracleHelper.getTWAPRatio(
            address(key_.pool),
            twapObservationWindow_
        );

        if (
            // Not necessary to use `isDeviatingWithBpsCheck()` as the checked is already performed in `addBunniToken`
            Deviation.isDeviating(
                reservesTokenRatio,
                twapTokenRatio,
                twapMaxDeviationBps_,
                TWAP_MAX_DEVIATION_BASE
            )
        ) {
            revert BunniSupply_PriceMismatch(
                address(key_.pool),
                twapTokenRatio,
                reservesTokenRatio
            );
        }

If deposited in the entire range the liquidity reserves ratio would track the spot price. But in the calculation of reservesRatio, the uncollected fees is also used. This can cause the calculation to deviate from the TWAP or approach TWAP even if the actual reserve ratio is differing from TWAP

    function getReservesRatio(BunniKey memory key_, BunniLens lens_) public view returns (uint256) {
        IUniswapV3Pool pool = key_.pool;
        uint8 token0Decimals = ERC20(pool.token0()).decimals();

        (uint112 reserve0, uint112 reserve1) = lens_.getReserves(key_);
        (uint256 fee0, uint256 fee1) = lens_.getUncollectedFees(key_);

        return (reserve1 + fee1).mulDiv(10 ** token0Decimals, reserve0 + fee0);
    }

Impact

Allows for manipulation of reserves beyond expected deviation: The actual reserve ratio can be increased or decreased by an attacker without deviating from TWAP if the uncollected fee amounts make up the ratio when added. It can happen naturally due to fee accrual or can be caused by an attacker by swapping. Depending on the amount of liquidity present in the pool and the amount of liquidity mintable by an attacker, it might become possible to achieve this at minimal loss since the fees calculated can include fees claimable by the attacker as Bunnihub allows any user to deposit.
Price and supply calculation can revert even without any pool manipulation.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/9c8df76dc9820b4c6605d2e1e6d87dcfa9e50070/bophades/src/libraries/UniswapV3/BunniHelper.sol#L47-L55

Tool used

Manual Review

Recommendation

Eliminate uncollected fees from the calculation

Duplicate of #49

sherlock-audit / 2023-11-olympus-judging

hash - Pool manipulation check in BunniHelper is flawed as uncollected fees is used #169