Description

In the BenqiDualOracle contract, the validateFeedPriceDeviation function is designed to validate a manually set price against the prices from the Edge and Chainlink oracles. However, if both the edgeFeed and chainlinkFeed are set to the zero address, the function effectively bypasses the validation process. This occurs because the function only performs validation checks if the feed addresses are nonzero. As a result, the manual price can be set without any comparison to oracle prices, potentially leading to inaccurate or manipulated price settings.

function validateFeedPriceDeviation(address asset, uint256 manualPrice) internal view {
    // Check Edge feed if available
    IAggregatorV2V3Interface edgeFeed = edgeOracle.getFeed(asset);
    if (address(edgeFeed) != address(0)) {
        uint256 feedPrice = getFeedPrice(edgeFeed);
        if (feedPrice > 0) {
            // Check for significant deviation (10x or 0.1x difference)
            if (manualPrice > feedPrice * 10 || feedPrice > manualPrice * 10) {
                revert ManualPriceDeviationError(asset, manualPrice, feedPrice);
            }
        }
    }

    // Check Chainlink feed if available
    IAggregatorV2V3Interface chainlinkFeed = chainlinkOracle.getFeed(asset);
    if (address(chainlinkFeed) != address(0)) {
        uint256 feedPrice = getFeedPrice(chainlinkFeed);
        if (feedPrice > 0) {
            // Check for significant deviation (10x or 0.1x difference)
            if (manualPrice > feedPrice * 10 || feedPrice > manualPrice * 10) {
                revert ManualPriceDeviationError(asset, manualPrice, feedPrice);
            }
        }
    }
}

Impact

The bypass of feed price validation could allow for manual prices to be set without any checks against oracle-provided prices. While the likelihood of both feeds being set to zero address is low, if it occurs, it could lead to incorrect price data being used within the system. This could affect the reliability and accuracy of the oracle system, although the overall impact is considered low due to the unlikelihood of this scenario.

Recommendations

To prevent the bypass of feed price validation, it is recommended to implement additional checks within the validateFeedPriceDeviation function. Specifically, the function should include logic to revert or throw an error if both edgeFeed and chainlinkFeed are set to the zero address. This will ensure that manual prices are always validated against at least one oracle price, maintaining the integrity and reliability of the oracle system.

Remediation

This issue has been acknowledged by Chaos Labs, and a fix was implemented in commit 967b0e3a↗.

Chaos Labs introduced a manualOverrideAllowed argument to several functions in the contract. This change is primarily related to the price configuration of BUSD. In order to maintain BUSD's price at a fixed value of $1.00 USD, it is necessary to set the price manually. Therefore, when the manualOverrideAllowed flag is explicitly set to true, the system is designed to permit price updates even if both oracle feeds are set to the zero address. This exception prevents the transaction from reverting in this specific scenario, ensuring compatibility with the intended manual price-setting mechanism.