sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

roguereddwarf - Missing Sequencer Uptime Feed check can cause unfair liquidations on Arbitrum #37

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

roguereddwarf

medium

Missing Sequencer Uptime Feed check can cause unfair liquidations on Arbitrum

Summary

When the Arbitrum sequencer is down and then comes back up, all Chainlink price updates will become available on Arbitrum within a very short time.

This leaves users no time to react to the price changes which can lead to unfair liquidations.

Vulnerability Detail

Chainlink explains their Sequencer Uptime Feeds here.

Quoting from the documentation:

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.

Users are still able in principle to avoid liquidations by interacting with the Arbitrum delayed inbox via L1, but this is out of reach for most users.

Impact

Users can get unfairly liquidated because they cannot react to price movements when the sequencer is down and when the sequencer comes back up, all price updates will immediately become available.

Code Snippet

This issue can be observed in both the ChainlinkOracle and ChainlinkFeedOracle, which do not make use of the sequencer uptime feed to check the status of the sequencer:

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/ChainlinkOracle.sol#L59-L64

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/ChainlinkFeedOracle.sol#L94-L97

Tool used

Manual Review

Recommendation

The Chainlink documentation contains an example for how to check the sequencer status: https://docs.chain.link/data-feeds/l2-sequencer-feeds

There can be a grace period when the sequencer comes back up for users to act on their collateral (increase collateral to avoid liquidation).

roguereddwarf commented 1 year ago

Escalate for 10 USDC

This issue is marked as a duplicate of #13.

This is wrong and I argue that my report is a legitimate Medium (speaking only of my report here, other dupes must be checked as well).

13 argues that outdated prices would be used when the sequencer is down.

The sponsor correctly explained that:

From our understanding, when the sequencer is down the prices can't be updated by the data feed so therefore settlement can't occur. This means that effectively each product is paused as no state changes can occur

My point on the other hand is that when the sequencer comes back up, all the old prices will be processed at once and users have no time to react to price changes, which can lead to unfair liquidations.

Therefore I had the suggestion for a grace period.

You can see a detailed explanation for my argument in the Aave V3 technical paper: https://github.com/aave/aave-v3-core/blob/master/techpaper/Aave_V3_Technical_Paper.pdf (section 4.6) 2023-06-30_09-38

sherlock-admin commented 1 year ago

Escalate for 10 USDC

This issue is marked as a duplicate of #13.

This is wrong and I argue that my report is a legitimate Medium (speaking only of my report here, other dupes must be checked as well).

13 argues that outdated prices would be used when the sequencer is down.

The sponsor correctly explained that:

From our understanding, when the sequencer is down the prices can't be updated by the data feed so therefore settlement can't occur. This means that effectively each product is paused as no state changes can occur

My point on the other hand is that when the sequencer comes back up, all the old prices will be processed at once and users have no time to react to price changes, which can lead to unfair liquidations.

Therefore I had the suggestion for a grace period.

You can see a detailed explanation for my argument in the Aave V3 technical paper: https://github.com/aave/aave-v3-core/blob/master/techpaper/Aave_V3_Technical_Paper.pdf (section 4.6) 2023-06-30_09-38

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

KenzoAgada commented 1 year ago

I think that this issue should have a similar fate to #190. If that escalation is accepted, this one should be accepted as well.

jacksanford1 commented 1 year ago

Believe #190 is trending towards being accepted. This issue has a different root case (Arbitrum sequencer down vs. protocol team pausing contracts) but the effect is the same (depositors can't salvage their position before they get liquidated).

I think it should be a valid Medium, even though the root case is a temporary freezing.

jacksanford1 commented 1 year ago

Result: Medium Unique Reasoning can be found in previous message.

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status: