sherlock-audit / 2023-02-gmx-judging

17 stars 11 forks source link

IllIllI - Missing checks for whether Arbitrum Sequencer is active #151

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

IllIllI

medium

Missing checks for whether Arbitrum Sequencer is active

Summary

Chainlink recommends that users using price oracles, check whether the Arbitrum Sequencer is active

Vulnerability Detail

If the sequencer goes down, the index oracles may have stale prices, since L2-submitted transactions (i.e. by the aggregating oracles) will not be processed.

Impact

Stale prices, e.g. if USDC were to de-peg while the sequencer is offline, would let people submit orders and if those people are also keepers, get execution prices that do not exist in reality.

Code Snippet

Chainlink oracles are used for some prices, but there are no sequencer oracles in use:

// File: gmx-synthetics/contracts/oracle/Oracle.sol : Oracle._setPricesFromPriceFeeds()   #1

569                IPriceFeed priceFeed = getPriceFeed(dataStore, token);
570    
571                (
572                    /* uint80 roundID */,
573                    int256 _price,
574                    /* uint256 startedAt */,
575                    /* uint256 timestamp */,
576                    /* uint80 answeredInRound */
577                ) = priceFeed.latestRoundData();
578    
579:               uint256 price = SafeCast.toUint256(_price);

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/oracle/Oracle.sol#L569-L579

Tool used

Manual Review

Recommendation

Use a chainlink oracle to determine whether the sequencer is offline or not, and don't allow orders to be executed while the sequencer is offline.

xvi10 commented 1 year ago

This is a valid issue, the risk will be reduced with a keeper network instead of a separate check

xvi10 commented 1 year ago

Additionally, if the sequencer is not producing blocks the oracle keepers should stop signing prices, when the sequencer resumes regular operation, the oracle keepers can continue signing prices for the most recent blocks at the latest fetched prices