sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

BLACK-PANDA-REACH - Lack of L2 Sequencer uptime check when getting oracle data on Arbitrum #211

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

BLACK-PANDA-REACH

medium

Lack of L2 Sequencer uptime check when getting oracle data on Arbitrum

Summary

Due to missing L2 Sequencer uptime checks when getting price data from Chainlink oracle on Arbitrum, users can get out of lost trades, due to being able to close positions on stale price data, while already knowing current prices.

Vulnerability Detail

Chainlink recommends checking whether the Arbitrum sequencer is active, when using oracle prices.

https://docs.chain.link/data-feeds#l2-sequencer-uptime-feeds

If the sequencer is down, oracles may return stale prices and versions, as transactions submitted to L2 (for example by aggregating oracles) will not be processed until the sequencer comes back online, while users still have a way to submit transactions to the L2 using delayed inbox.

https://developer.arbitrum.io/sequencer#unhappyuncommon-case-sequencer-isnt-doing-its-job

Proof of Concept

image

Let's imagine the price action and sequencer downtime as shown on the graph. Alice has a taker long position opened on Arbitrum on the ETH/USD market, last updated to oracle version x.

  1. Sequencer goes down
  2. Price drops rapidly (against Alice's prediction)
  3. Alice sends her transaction to close her take position through a delayed inbox to Arbitrum network. Settle version for her prePosition created through this action would be x+1, even though more oracle updates have been performed during this time (current version= x+2).
  4. Price drops further
  5. Alice's transaction is included via forceInclusion and a pre-position is scheduled to settle at version x+1
  6. Sequencer goes back up and Product is settled. The pre-position is settled at version x+1, even though the transaction was submitted after version x+2 (so the settle version should be x+3) giving Alice an unfairly favourable settle price.

Impact

As by the Perennial documentation:

On a continuous, on-going basis, LPs and traders settle up; the losing side of the trade pays the winning side.

Users can get out of lost trades using stale oracle prices, creating a loss for the winning side.

Assigning medium severity as by Sherlock docs:

Medium: There is a viable scenario (even if unlikely) that could cause the protocol to enter a state where a material amount of funds can be lost.

Code Snippet

Getting oracle data https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial-oracle/contracts/types/ChainlinkRegistry.sol#L34-L38

Creating pre-position based on latest synced oracle version https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/product/Product.sol#L261

Calculating pre-position's settle oracle version https://github.com/sherlock-audit/2023-05-perennial/blob/0f73469508a4cd3d90b382eac2112f012a5a9852/perennial-mono/packages/perennial/contracts/interfaces/types/PrePosition.sol#L133-L136

Tool used

Manual Review

Recommendation

Use sequencer oracle to determine if sequencer is active or not and prevent executing trades during sequencer downtime.

Duplicate of #13