sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Bauchibred - Oracles being unpausable could breed more disadvantages than advantages #86

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bauchibred

medium

Oracles being unpausable could breed more disadvantages than advantages

Summary

There are no mechanisms for the price data feeds to be paused if an emergency situation on the data provider happens.

Vulnerability Detail

The protocol implements a functionality that is used to prevent outdated prices received by users. However, in the function, the twap period used (in this case secondsAgo )could be a very low figure that might be influenced by an attacker. And this in turn is used to get a price from the oracle

The same secondsAgo is allowed to be as low as 10 seconds which is a very low value and could definitely be influenced by an attacker

UniswapV3Oracle.getprice();

 /// @notice Return the USD based price of the given input, multiplied by 10**18.
    /// @dev Fair LP Price Formula => Price = 2 * (sqrt(r0 x r1) x sqrt(p0 x p1)) / total supply
    /// @param pair The Uniswap pair to check the value.
    function getPrice(address pair) external view override returns (uint256) {
        IUniswapV2Pair pool = IUniswapV2Pair(pair);
        uint256 totalSupply = pool.totalSupply();
        if (totalSupply == 0) return 0;

        address token0 = pool.token0();
        address token1 = pool.token1();

        (uint256 r0, uint256 r1, ) = pool.getReserves();
        uint256 px0 = base.getPrice(token0);
        uint256 px1 = base.getPrice(token1);
        uint256 t0Decimal = IERC20Metadata(token0).decimals();
        uint256 t1Decimal = IERC20Metadata(token1).decimals();
        uint256 sqrtK = BBMath.sqrt(
            r0 * r1 * 10 ** (36 - t0Decimal - t1Decimal)
        );

        return (2 * sqrtK * BBMath.sqrt(px0 * px1)) / totalSupply;
    }
}

UniswapV2Oracle.getprice():

    /// @notice Return the USD based price of the given input, multiplied by 10**18.
    /// @dev Fair LP Price Formula => Price = 2 * (sqrt(r0 x r1) x sqrt(p0 x p1)) / total supply
    /// @param pair The Uniswap pair to check the value.
    function getPrice(address pair) external view override returns (uint256) {
        IUniswapV2Pair pool = IUniswapV2Pair(pair);
        uint256 totalSupply = pool.totalSupply();
        if (totalSupply == 0) return 0;

        address token0 = pool.token0();
        address token1 = pool.token1();

        (uint256 r0, uint256 r1, ) = pool.getReserves();
        uint256 px0 = base.getPrice(token0);
        uint256 px1 = base.getPrice(token1);
        uint256 t0Decimal = IERC20Metadata(token0).decimals();
        uint256 t1Decimal = IERC20Metadata(token1).decimals();
        uint256 sqrtK = BBMath.sqrt(
            r0 * r1 * 10 ** (36 - t0Decimal - t1Decimal)
        );

        return (2 * sqrtK * BBMath.sqrt(px0 * px1)) / totalSupply;
    }

IchiVaultOracle.getprice():


  /**
     * @notice Return vault token price in USD, with 18 decimals of precision.
     * @param token The vault token to get the price of.
     * @return price USD price of token in 18 decimal
     */
    function getPrice(address token) external view override returns (uint256) {
        IICHIVault vault = IICHIVault(token);
        uint256 totalSupply = vault.totalSupply();
        if (totalSupply == 0) return 0;

        address token0 = vault.token0();
        address token1 = vault.token1();

        // Check price manipulations on Uni V3 pool by flashloan attack
        uint256 spotPrice = spotPrice0InToken1(vault);
        uint256 twapPrice = twapPrice0InToken1(vault);
        uint256 maxPriceDeviation = maxPriceDeviations[token0];
        if (!_isValidPrices(spotPrice, twapPrice, maxPriceDeviation))
            revert Errors.EXCEED_DEVIATION();

        // Total reserve / total supply
        (uint256 r0, uint256 r1) = vault.getTotalAmounts();
        uint256 px0 = base.getPrice(address(token0));
        uint256 px1 = base.getPrice(address(token1));
        uint256 t0Decimal = IERC20Metadata(token0).decimals();
        uint256 t1Decimal = IERC20Metadata(token1).decimals();

        uint256 totalReserve = (r0 * px0) /
            10 ** t0Decimal +
            (r1 * px1) /
            10 ** t1Decimal;

        return (totalReserve * 10 ** vault.decimals()) / totalSupply;
    }
}

Impact

In case of emergencies, take what happened to UST for example, querying to the oracle can't be stopped and oracle would probably return the wrong values and break a lot of protocol's functionality or even open windows for other users to take advantage on the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/oracle/UniswapV3AdapterOracle.sol#L57-L90

https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/oracle/UniswapV2Oracle.sol#L30-L51

https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/oracle/IchiVaultOracle.sol#L105-L138

Tool used

Manual Review

Recommendation

Though this might introduce more centralization risks to the users from admin, a mechanism should be introduced so that oracles could be paused when there is an emergency