sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

0xvj - Chainlink’s latestRoundData might return stale or incorrect results #225

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xvj

medium

Chainlink’s latestRoundData might return stale or incorrect results

Summary

In Oracle.sol getUnderlyingPrice() function you are using latestRoundData function, but there is no check if the return value indicates stale data.

Vulnerability Detail

The Oracle.sol calls out to a Chainlink oracle receiving the latestRoundData(). If there is a problem with Chainlink starting a new round and finding consensus on the new value for the oracle (e.g. Chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the chainlink system) consumers of this contract may continue using outdated stale or incorrect data (if oracles are unable to submit no new round is started).

Impact

This could lead to stale prices according to the Chainlink documentation: https://docs.chain.link/data-feeds/price-feeds/historical-data

Code Snippet

    function getUnderlyingPrice(address underlying)
        virtual
        external
        view
        returns(int256 answer)
    {
        if (stablePrice[underlying] != 0) {
            return stablePrice[underlying];
        }
        (,answer,,,) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();
        require(answer > 0, "Oracle.getUnderlyingPrice.non_positive");
        answer /= 100;
    }

https://github.com/hubble-exchange/hubble-protocol/blob/d89714101dd3494b132a3e3f9fed9aca4e19aef6/contracts/Oracle.sol#L24C1-L36C6

Tool used

Manual Review

Recommendation

    function getUnderlyingPrice(address underlying)
        virtual
        external
        view
        returns(int256 answer)
    {
        if (stablePrice[underlying] != 0) {
            return stablePrice[underlying];
        }
-       (,answer,,,) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();
+       (uint80 roundID, int256 answer, uint256 timestamp, uint256 updatedAt, ) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();
+       require(updatedAt >= roundID, "Stale price");
+       require(timestamp != 0,"Round not complete");
        require(answer > 0, "Oracle.getUnderlyingPrice.non_positive");

+       if (updatedAt < block.timestamp - maxDelayTime)
+           revert PRICE_OUTDATED(_token);    
        answer /= 100;
}

Duplicate of #18