Chainlink's latestRoundData might return stale or incorrect results

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/core/PriceFeed.sol#L66-L66

Vulnerability details

Impact

In many contracts , we are using latestRoundData, 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

Proof of Concept

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/core/PriceFeed.sol#L66-L66
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/core/PriceFeed.sol#L70-L70
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#L74-L74
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#L78-L78
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/BalancerV2LPOracle.sol#L101-L102
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/GUniLPOracle.sol#L103-L104

Tools Used

None

Recommended Mitigation Steps

For example

      (uint80 roundID, int256 answer, , uint256 timestamp, uint80 answeredInRound) = eurOracle.latestRoundData();
      require(answeredInRound >= roundID, "Stale price");
      require(timestamp != 0,"Round not complete");
      require(answer > 0,"Chainlink answer reporting 0");

[m19]

We disagree because of the following reasons:

• In all the PriceFeed contracts staleness is correctly checked using the updatedAt field. Checking for both roundID and answeredInRound is redundant because answeredInRound and roundId are always the same in the latest Chainlink agggregators contracts. This is a duplicate of Should check return data from chainlink aggregators #1
• In the Oracle contracts it's intended to return the oldest the updatedAt because the consumer of an Oracle contract can then decide what to do with it, different contracts might have different requirements on what is considered stale, as the Oracle contracts are consumed by our PriceFeed