Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/oracle/PriceFeed.sol#L84

Vulnerability details

Impact

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. The result is that if an asset experiences a huge drop in value (i.e. crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset. This would allow user to continue borrowing with the asset but at the wrong price. This will therefore cause the protocol or the user to lose funds

Proof of Concept

function _getPrice(AggregatorV3Interface aggregator, uint256 stalePriceInterval) internal view returns (uint256) {
        // slither-disable-next-line unused-return
        (, int256 price,, uint256 updatedAt,) = aggregator.latestRoundData();

        if (price <= 0) revert Errors.INVALID_PRICE(address(aggregator), price);
        if (block.timestamp - updatedAt > stalePriceInterval) {
            revert Errors.STALE_PRICE(address(aggregator), updatedAt);
        }

        return SafeCast.toUint256(price);
    }
}

From the above, it is clearly seen that there is only a check for price to be non-negative and fresh, but no check for price within an acceptable range. This could result to a big disaster when Alice wants to borrow an asset and ends up borrowing it at a wrong price. This would occur if an asset experiences a huge drop in value (i.e. crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset.

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/oracle/PriceFeed.sol#L84

Tools Used

Recommended Mitigation Steps

Implement the proper check for each asset. It must revert if the price is outside the acceptable range.

function _getPrice(AggregatorV3Interface aggregator, uint256 stalePriceInterval) internal view returns (uint256) {
    // slither-disable-next-line unused-return
    (, int256 price,, uint256 updatedAt,) = aggregator.latestRoundData();
require(price >= minPrice && price <= maxPrice, "invalid price" (address(aggregator), price); // 
    if (block.timestamp - updatedAt > stalePriceInterval) {
        revert Errors.STALE_PRICE(address(aggregator), updatedAt);
    }
    return SafeCast.toUint256(price);
}

2.Or Implement a protective measure to mitigate this potential risk. For example, enclose any oracle get price function within a try-catch block, and incorporate fallback logic within the catch block to handle scenarios where access to the Chainlink oracle data feed is denied. Or use second oracle for backup situation.

Assessed type

Oracle

code-423n4 / 2024-06-size-validation

PriceOracle will use the wrong price if the Chainlink registry returns price outside min/max range. #709