code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

Chainlink oracle does not check that the L2 sequencer is offline #133

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Oracles/ChainlinkOracle.sol#L100-L103 https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Oracles/ChainlinkCompositeOracle.sol#L189-L190

Vulnerability details

Impact

When chainlink oracle is deployed to the L2 network, it relies on the L2 sequencer for price updates. When the sequencer goes offline, the oracle price cannot be updated in time, and the stale price is obtained, which will affect the operation of the protocol, such as lending and liquidation, and may also cause fund losses to users. As the chainlink documentation states, you should verify that the sequencer is offline before getting the price:

If you are using Chainlink Data Feeds on L2 networks like Arbitrum, Optimism, and Metis, you must also check the latest answer from the L2 Sequencer Uptime Feed to ensure that the data is accurate in the event of an L2 sequencer outage. See the L2 Sequencer Uptime Feeds page to learn how to use Data Feeds on L2 networks.

Proof of Concept

        (, int256 answer, , uint256 updatedAt, ) = AggregatorV3Interface(feed)
            .latestRoundData();
        require(answer > 0, "Chainlink price cannot be lower than 0");
        require(updatedAt != 0, "Round is in incompleted state");

Note that this is not a repeat of oracle's price freshness problem, where token prices fluctuate when the sequencer goes offline but cannot be updated. At this time, although the previous price has not expired, it is not the latest price, then users may use the price difference for arbitrage, or innocent users may suffer losses:

  1. The tokenA's price is 100, and the tokenB's price is 1
  2. Sequencer is down, L2 predictor cannot be updated, and the tokenB price increases to $2
  3. Users can still borrow at $1, and if they can borrow 60, resulting in a protocol loss

Tools Used

Manual review

Recommended Mitigation Steps

There are two things to note here:

  1. Check if the L2 sequencer is offline before getting the price
  2. It is necessary to wait for a cooling period after the recovery of the sequencer to perform protocol manipulation, and give users a certain buffer to repay and other operations
    // Check the sequencer status and return the latest data
    function getLatestData() public view returns (int) {
        // prettier-ignore
        (
            /*uint80 roundID*/,
            int256 answer,
            uint256 startedAt,
            /*uint256 updatedAt*/,
            /*uint80 answeredInRound*/
        ) = sequencerUptimeFeed.latestRoundData();

        // Answer == 0: Sequencer is up
        // Answer == 1: Sequencer is down
        bool isSequencerUp = answer == 0;
        if (!isSequencerUp) {
            revert SequencerDown();
        }

        // Make sure the grace period has passed after the
        // sequencer is back up.
        uint256 timeSinceUp = block.timestamp - startedAt;
        if (timeSinceUp <= GRACE_PERIOD_TIME) {
            revert GracePeriodNotOver();
        }

        // prettier-ignore
        (
            /*uint80 roundID*/,
            int data,
            /*uint startedAt*/,
            /*uint timeStamp*/,
            /*uint80 answeredInRound*/
        ) = dataFeed.latestRoundData();

        return data;
    }

Assessed type

Invalid Validation

code423n4 commented 1 year ago

Withdrawn by kutugu