Chainlink's `latestRoundData` might return stale results

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/PriceFeed.sol#L578-L589

function _badChainlinkResponse(ChainlinkResponse memory _response) internal view returns (bool) {
    // Check for response call reverted
    if (!_response.success) {return true;}
    // Check for an invalid roundId that is 0
    if (_response.roundId == 0) {return true;}
    // Check for an invalid timeStamp that is 0, or in the future
    if (_response.timestamp == 0 || _response.timestamp > block.timestamp) {return true;}
    // Check for non-positive price
    if (_response.answer <= 0) {return true;}

    return false;
}

On PriceFeed.sol, 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

Recommendation

Consider adding missing checks for stale data.

For example:

function _badChainlinkResponse(ChainlinkResponse memory _response) internal view returns (bool) {
    // Check for response call reverted
    if (!_response.success) {return true;}
    // Check for an invalid roundId that is 0
    if (_response.roundId == 0) {return true;}
    // Check for stale price
    if (_response.answeredInRound < _response.roundID) {return true;}
    // Check for an invalid timeStamp that is 0, or in the future
    if (_response.timestamp == 0 || _response.timestamp > block.timestamp) {return true;}
    // Check for non-positive price
    if (_response.answer <= 0) {return true;}

    return false;
}

[kingyetifinance]

Duplicate #91