sherlock-audit / 2023-12-flatmoney-judging

9 stars 8 forks source link

shaka - It is not checked whether the sequencer is down when fetching the price from Chainlink #218

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

shaka

medium

It is not checked whether the sequencer is down when fetching the price from Chainlink

Summary

It is not checked whether the sequencer is down when fetching the price from Chainlink.

Vulnerability Detail

Chainlink's price feeds in layer 2 networks are updated through the sequencer, which can become unavailable.

The Chainlink documentation states the following regarding its use in layer 2 networks:

To help your applications identify when the sequencer is unavailable, you can use a data feed that tracks the last known status of the sequencer at a given point in time. This helps you prevent mass liquidations by providing a grace period to allow customers to react to such an event.

The implementation of OracleModule.sol:_getOnchainPrice() lacks this validation, which can result in the protocol using an outdated price.

Impact

If the sequencer in Base goes down the oracle will return stale prices. In the case that the onchain oracle price is used because the offchain oracle price is invalid, the wrong price will be used for the calculations of the PnL and the liquidations.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L141-L157

Tool used

Manual review

Recommendation

+   function checkSequencerUptime() internal view returns (bool) {
+       (, int256 answer, uint256 startedAt,,) = sequencerUptimeFeed.latestRoundData();
+       require(block.timestamp - startedAt > GRACE_PERIOD_TIME && answer == 0, "Sequencer is down");
+   }

    function _getOnchainPrice() internal view returns (uint256 price, uint256 timestamp) {
        IChainlinkAggregatorV3 oracle = onchainOracle.oracleContract;
        if (address(oracle) == address(0)) revert FlatcoinErrors.ZeroAddress("oracle");
+       checkSequencerUptime();

        (, int256 _price, , uint256 updatedAt, ) = oracle.latestRoundData();
        timestamp = updatedAt;
        // check Chainlink oracle price updated within `maxAge` time.
        if (block.timestamp > timestamp + onchainOracle.maxAge)
            revert FlatcoinErrors.PriceStale(FlatcoinErrors.PriceSource.OnChain);

        if (_price > 0) {
            price = uint256(_price) * (10 ** 10); // convert Chainlink oracle decimals 8 -> 18
        } else {
            // Issue with onchain oracle indicates a serious problem
            revert FlatcoinErrors.PriceInvalid(FlatcoinErrors.PriceSource.OnChain);
        }
    }

Duplicate of #27

sherlock-admin commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid