Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Unchecked Chainlink's `latestRoundData` return #327

Closed rotcivegaf closed 1 year ago

rotcivegaf commented 1 year ago

Git branch: H01

Affected smart contract

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

Description

The FujiOracle contract uses Chainlink's latestRoundData API, but not check the returned data According to the Chainlink documentation, this could lead to stale prices:

The result of latestRoundData API will be used across various functions, therefore, a stale price from Chainlink can lead to loss of funds to end-users.

Attack scenario

The BorrowingVault contract use this returned data to:

All these functions are critical, in case of the returned price is stale could lead to a big loss for users, such as early liquidation

Recommendation

Consider adding the missing checks for stale data, for example:

    (uint80 roundID, int256 latestPrice,, uint256 updatedAt, uint80 answeredInRound) = IAggregatorV3(usdPriceFeeds[asset]).latestRoundData();

    if (latestPrice <= 0) {
      revert FujiOracle__negativePrice(latestPrice);
    }
    if (answeredInRound < roundID) {
      revert FujiOracle__stalePrice();
    }
    if (updatedAt == 0) {
      revert FujiOracle__roundNotComplete();
    }
    if (updatedAt < block.timestamp - observationFrequency) {
      revert FujiOracle__outdatedPrice();
    }
0xdcota commented 1 year ago

This issue will be closed as it relates closely to #297.