santipu_ - Lack of checks to avoid stale prices from Chainlink oracle

[sherlock-admin]

santipu_

medium

Lack of checks to avoid stale prices from Chainlink oracle

Summary

getPriceFromChainlink function is not checking the returned values of Chainlink Registry to ensure the prices are fresh and not stale.

Vulnerability Detail

When getting prices from Chainlink oracles, it's necessary to ensure the prices are fresh and not stale, and getPriceFromChainlink function from PriceOracle contract is not checking that.

/**
 * @notice Get price from Chainlink.
 * @param base The base asset
 * @param quote The quote asset
 * @return The price
 */
function getPriceFromChainlink(address base, address quote) internal view returns (uint256) {
    (, int256 price,,,) = registry.latestRoundData(base, quote);
    require(price > 0, "invalid price"); // @audit No checks for stale prices

    // Extend the decimals to 1e18.
    return uint256(price) * 10 ** (18 - uint256(registry.decimals(base, quote)));
}

Every Chainlink price feed has a different heartbeat, meaning it has to be updated every x amount of seconds depending on the feed, here some examples:

When getting prices from feeds, we have to ensure that the answer given by the oracle is fresh, meaning the answer given hasn't been updated more than a certain time ago. For ETH/USD oracle, the heartbeat is of 3600 seconds, meaning the oracle must be updated at least every 1 hour.

Related reports:

• SPYBOY - Chainlink's latestRoundData return stale or incorrect result 2023-02-blueberry-judging#94
• Should check return data from Chainlink aggregators code-423n4/2021-05-fairside-findings#70

Impact

If the Chainlink oracle is returning stale prices, the protocol will use the wrong values to make decisions so it could lead to false liquidations or over-borrowing.

Code Snippet

https://github.com/sherlock-audit/2023-05-ironbank/blob/main/ib-v2/src/protocol/oracle/PriceOracle.sol#L60-L72

Tool used

Manual Review

Recommendation

It's recommended to check updatedAt value returned from Chainlink against some maxDelayTime value set in the contract depending of the feed used.

Example:

    function getPriceFromChainlink(address base, address quote) internal view returns (uint256) {
        (, int256 price,, uint256 updatedAt,) = registry.latestRoundData(base, quote);
        require(price > 0, "invalid price");
+       if(updatedAt < block.timestamp - maxDelayTime) revert(); // @audit Ensuring fresh prices

        // Extend the decimals to 1e18.
        return uint256(price) * 10 ** (18 - uint256(registry.decimals(base, quote)));
    }

Duplicate of #9