oracle: data can be outdated

[WelToHackerLand]

Title

Oracle data can be outdated

Affected smart contract

https://github.com/Fujicracy/fuji-v2/blob/1b939ec84af137db430fc2aa1b4c6f15e5254003/packages/protocol/src/FujiOracle.sol#L113

Description

The lastRoundData()'s parameters according to Chainlink are the following:

function latestRoundData() external view
    returns (
        uint80 roundId,             //  The round ID.
        int256 answer,              //  The price.
        uint256 startedAt,          //  Timestamp of when the round started.
        uint256 updatedAt,          //  Timestamp of when the round was updated.
        uint80 answeredInRound      //  The round ID of the round in which the answer was computed.
    )

But function FujiOracle._getUSDPrice() just get the parameter price, and hasn't checked the others.

function _getUSDPrice(address asset) internal view returns (uint256 price) {
    if (usdPriceFeeds[asset] == address(0)) {
      revert FujiOracle__noPriceFeed();
    }

    (, int256 latestPrice,,,) = IAggregatorV3(usdPriceFeeds[asset]).latestRoundData();

    price = uint256(latestPrice);
  }

A strong reliance on the price feeds has to be also monitored as recommended on the Risk Mitigation section. There are several reasons why a data feed may fail such as unforeseen market events, volatile market conditions, degraded performance of infrastructure, chains, or networks, upstream data providers outage, malicious activities from third parties among others.

Chainlink recommends using their data feeds along with some controls to prevent mismatches with the retrieved data. Along some recommendations, the feed can include circuit breakers (for extreme price events), contract update delays (to ensure that the injected data into the protocol is fresh enough), manual kill-switches (to cease connection in case of found bug or vulnerability in an upstream contract), monitoring (control the deviation of the data) and soak testing (of the price feeds).

The retrieved price of the BorrowingVault.debtAsset() and BorrowingVault.asset() can be outdated and used anyways as a valid data because no timestamp tolerance of the update source time is checked while storing the return parameters of latestRoundData() inside _latestAnswer64x64 as recommended by Chainlink. The usage of outdated data can impact on how the Payment terminals work regarding pricing calculation and value measurement.

Attack scenario

Since the return price from oracle can be outdated, it will make the function BorrowingVault._computeMaxBorrow() return the maximum amount that user can borrow. User can borrow more than expected / less than expected, in both way it will affect badly to user and protocol. Link to test PR: https://github.com/Fujicracy/fuji-v2/pull/301

Recommendation

Follow the Chainlink's recommendation: https://docs.chain.link/docs/data-feeds/#check-the-timestamp-of-the-latest-answer It is recommended both to add also a tolerance that compares the updatedAt return timestamp from latestRoundData() with the current block timestamp and ensure that the priceFeed is being updated with the required frequency. For example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = ETH_CHAINLINK.latestRoundData();
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");