[WP-M6] Chainlink's latestRoundData might return stale results

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/Oracle.sol#L24-L35

Vulnerability details

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/Oracle.sol#L24-L35

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

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/Oracle.sol#L64-L69

(uint80 round, uint256 latestPrice, uint256 latestTimestamp) = getLatestRoundData(aggregator);
uint256 baseTimestamp = _blockTimestamp() - intervalInSeconds;
// if latest updated timestamp is earlier than target timestamp, return the latest price.
if (latestTimestamp < baseTimestamp || round == 0) {
    return formatPrice(latestPrice);
}

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L460-L468

function _getLiquidationInfo(address trader, uint idx) internal view returns (LiquidationBuffer memory buffer) {
    require(idx > VUSD_IDX && idx < supportedCollateral.length, "collateral not seizable");
    (buffer.status, buffer.repayAble, buffer.incentivePerDollar) = isLiquidatable(trader, false);
    if (buffer.status == IMarginAccount.LiquidationStatus.IS_LIQUIDATABLE) {
        Collateral memory coll = supportedCollateral[idx];
        buffer.priceCollateral = oracle.getUnderlyingPrice(address(coll.token)).toUint256();
        buffer.decimals = coll.decimals;
    }
}

On Oracle.sol, we are using Chainlink's latestRoundData API, but there is no check if the return value indicates stale data. This could lead to stale prices according to the Chainlink documentation:

• https://docs.chain.link/docs/historical-price-data/#historical-rounds
• https://docs.chain.link/docs/faq/#how-can-i-check-if-the-answer-to-a-round-is-being-carried-over-from-a-previous-round

The result of latestRoundData API will be used for MarginAccount.sol#liquidateExactRepay(), therefore, a stale price from Chainlink can lead to loss of funds to end-users.

Recommendation

Consider adding missing checks for stale data.

For example:

(uint80 roundID ,answer,, uint256 timestamp, uint80 answeredInRound) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();

require(answer > 0, "Chainlink price <= 0"); 
require(answeredInRound >= roundID, "Stale price");
require(timestamp != 0, "Round not complete");

[atvanguard]

Duplicate of #46