0xmuxyz - Lack of validation to check whether or not the return value would be a stale price data

[sherlock-admin]

0xmuxyz

medium

Lack of validation to check whether or not the return value would be a stale price data

Summary

Within the Oracle#getUnderlyingPrice(), the AggregatorV3Interface#latestRoundData(), which is made by the Chainlink, would be called to get the latest underlying token price data.

However, there is no validation to check whether or not the return value would be a stale price data.
This could lead to retrieving the stale prices.

Vulnerability Detail

Within the Oracle#getUnderlyingPrice(), the AggregatorV3Interface#latestRoundData(), which is made by the Chainlink, would be called to get the latest underlying token price data like this:
https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/Oracle.sol#L33-L35

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

However, there is no validation to check whether or not the return value would be a stale price data.
This could lead to retrieving the stale prices.

Here is a related-report that is previously reported:
code-423n4/2021-05-fairside-findings#70

Impact

The Oracle#getUnderlyingPrice() above would be called in the multiple functions like below:

• Within the AMM#getOptimalPnl(), the Oracle#getUnderlyingPrice() would be called like this:
  https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L340

• Within the AMM#lastPrice(), the Oracle#getUnderlyingPrice() would be called like this:
  https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L394

• Within the the AMM#getUnderlyingPrice(), the Oracle#getUnderlyingPrice() would be called like this:
  https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L400

Thus, a stale price data may be retrieved and used in these functions above.

Code Snippet

• https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/Oracle.sol#L33-L35
• https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L340
• https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L394
• https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L400

Tool used

Manual Review

Recommendation

Within the Oracle#getUnderlyingPrice(), consider adding the validations in order to check whether or not the retrieved-price via the AggregatorV3Interface#latestRoundData() would be a stale price data like this:

    function getUnderlyingPrice(address underlying)
        virtual
        external
        view
        returns(int256 answer)
    {
        if (stablePrice[underlying] != 0) {
            return stablePrice[underlying];
        }
-       (,answer,,,) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();
+       (uint80 roundID, 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");
        answer /= 100;
    }

Duplicate of #18