No check if Arbitrum/Optimism L2 sequencer is down in Chainlink feeds `PriceOracle.sol`

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/main/src/PriceOracle.sol#L122

Vulnerability details

Description

Chainlink recommends that all Optimistic L2 oracles consult the Sequencer Uptime Feed to ensure that the sequencer is live before trusting the data returned by the oracle.

If the Arbitrum Sequencer goes down, oracle data will not be kept up to date, and thus could become stale. However, users are able to continue to interact with the protocol directly through the L1 optimistic rollup contract. You can review Chainlink docs on L2 Sequencer Uptime Feeds for more details on this.

This protocol uses getPriceOracle() everywhere and it sub call to PriceOracle.sol#latestRoundData() As a result, users may be able to use the protocol while Oracle feeds are stale. This could cause many problems, example:

A user has a loan with 100 tokens, valued at 1 ETH each, and no borrows The Arbitrum sequencer goes down temporarily While it's down, the price of the token falls to 0.5 ETH each The current value of the user's loan is 50 ETH, so the liquidators should be able to liquidate this position because it is unhealthy However, due to the stale price, the protocol lets the user keep borrowing.

Impact

If the Arbitrum sequencer goes down, the protocol will allow users to continue to operate at the previous (stale) prices. this will brack all the core functionality in the protocol.

Tools Used

Manual Review - Solodit

Recommended Mitigation Steps

It is recommended to follow the code example of Chainlink: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code or just add theis check:

File: PriceOracle.sol

function getAssetPriceFromChainlink(address asset) public view returns (uint256) {
   AggregatorV2V3Interface sourceAgg = assetChainlinkAggregators[asset];
   require(address(sourceAgg) != address(0), Errors.ASSET_AGGREGATOR_NOT_EXIST);
   if (!isSequencerActive()) revert Errors.L2SequencerUnavailable();

    ...
}

function isSequencerActive() internal view returns (bool) {
    (, int256 answer, uint256 startedAt,,) = sequencer.latestRoundData();
    if (block.timestamp - startedAt <= GRACE_PERIOD_TIME || answer == 1)
        return false;
    return true;
}

Assessed type

Oracle

[c4-judge]

MarioPoneder marked the issue as primary issue

[c4-judge]

MarioPoneder marked the issue as satisfactory

[c4-judge]

MarioPoneder marked the issue as selected for report

[thorseldon]

Checking the stale interval or grace period of oracle price, it's maybe better do this as suggested, but it's hard to set or predict the appropriate interval time.

[thorseldon]

We suggest adjust the severity level to informative.