code-423n4 / 2024-05-predy-findings

10 stars 9 forks source link

Chainlink aggregator does not handle minAnswer/maxAnswer circuit breakers #30

Closed c4-bot-7 closed 4 months ago

c4-bot-7 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/PriceFeed.sol#L46 https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/PositionCalculator.sol#L141-L149

Vulnerability details

Impact

Price feed for tokens may be incorrect due to the circuit breaker mechanism in Chainlink, causing inaccurate calculation for userPosition (vault) value.

Bug Description

If the priceFeed field is set for a PairStatus, when calculating the vaule of a userPosition (vault), it will use the baseToken/quoteToken value from priceFeed oracle.

The PriceFeed.sol implementation uses Chainlink Aggregator to calculate the value for the quoteToken. However, it does not check for minAnswer/maxAnswer price. This means if the circuit breaker mechanism is implemented for this price feed, the return value may be incorrect.

A simple introduction to the circuit breaker mechanism in Chainlink: For some data feed aggregators, there is a minAnswer/maxAnswer check, that if the oracle price feed falls out of range, it will simply return the minAnswer/maxAnswer. For example, if the price falls below minAnswer, the oracle will simply return minAnswer. See https://docs.chain.link/data-feeds#check-the-latest-answer-against-reasonable-limits for more details.

From the contest README, we know the Predy protocol supports DAI on the Arbitrum chain. We can lookup the data feed for DAI/USD https://arbiscan.io/address/0xc5C8E77B397E531B8EC06BFb0048328B30E9eCfB#readContract, and fetch its aggregator address https://arbiscan.io/address/0xFc06bB03a9e1D8033f87eA6A682cbd65477A43b9#readContract. We can check that the minAnswer == 1e6, maxAnswer == 1e11, with a decimals == 8. This means the DAI/USD price is bounded in the range [0.01, 1000].

In history, the circuit breaker caused this issue https://rekt.news/venus-blizz-rekt/ during the LUNA crash.

PositionCalculator.sol

    function calculateMinMargin(
        DataType.PairStatus memory pairStatus,
        DataType.Vault memory vault,
        DataType.FeeAmount memory feeAmount
    ) internal view returns (int256 minMargin, int256 vaultValue, bool hasPosition, uint256 sqrtOraclePrice) {
        ...

>       sqrtOraclePrice = getSqrtIndexPrice(pairStatus);

        (minValue, vaultValue, debtValue, hasPosition) = calculateMinValue(
            vault.margin,
            getPositionWithFeeAmount(vault.openPosition, feeAmount),
            sqrtOraclePrice,
            pairStatus.riskParams.riskRatio
        );
        ...
    }

    function getSqrtIndexPrice(DataType.PairStatus memory pairStatus) internal view returns (uint256 sqrtPriceX96) {
>       if (pairStatus.priceFeed != address(0)) {
>           return PriceFeed(pairStatus.priceFeed).getSqrtPrice();
>       } else {
            return UniHelper.convertSqrtPrice(
                UniHelper.getSqrtTWAP(pairStatus.sqrtAssetStatus.uniswapPool), pairStatus.isQuoteZero
            );
        }
    }

PriceFeed.sol

    /// @notice This function returns the square root of the baseToken price quoted in quoteToken.
    function getSqrtPrice() external view returns (uint256 sqrtPrice) {
>       (, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();

        IPyth.Price memory basePrice = IPyth(_pyth).getPriceNoOlderThan(_priceId, VALID_TIME_PERIOD);

        require(basePrice.expo == -8, "INVALID_EXP");

        require(quoteAnswer > 0 && basePrice.price > 0);

        uint256 price = uint256(int256(basePrice.price)) * Constants.Q96 / uint256(quoteAnswer);
        price = price * Constants.Q96 / _decimalsDiff;

        sqrtPrice = FixedPointMathLib.sqrt(price);
    }

Proof of Concept

N/A

Tools Used

Manual review

Recommended Mitigation Steps

Add a minPrice/maxPrice check, and revert if is out of bounds. An example is:

    (, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();
    if (quoteAnswer > maxPrice || quoteAnswer < minPrice) revert();

Assessed type

Oracle

c4-judge commented 3 months ago

alex-ppg marked the issue as unsatisfactory: Invalid