DSC does not implement a check for the sequencer uptime which could lead to unfair liquidations

[codehawks-bot]

DSC does not implement a check for the sequencer uptime which could lead to unfair liquidations

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L229-L262

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L310

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/DSCEngine.sol#L236

Summary

Issue exist since DSC is not going to be deployed only on mainnet, and in the case where it gets deployed on L2s, say arbitrum, if 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 eventually leaves users no time to react to the price changes which can lead to unfair liquidations.

Vulnerability Details

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.

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

Now a call to the getAccountCollateralValue() function would be easily affected as it leads to a call to query the chainlink price via getUsdValue(), and note that this function is used while getting a user's account information, which would be queried while checking for a users health factor(via healthfactor()), all this essentially lead to the check for user's health factor in this line of the liquidate() function being unfairly passed, since user didn't have ample time to react to the sequencer coming back up.

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.

Tools Used

Manual Audit

Recommend Mitigation

The Chainlink documentation contains an example for how to check the sequencer status, seen here

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

Additional Note

This issue is not rooted on the fact that wrong price could be used while the sequencer is down(A pretty popular vulnerabily in the web3 security space of "L2 sequencer could be down"), but rather the fact 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, which is why the suggestion of a grace period was made. Additionally the Aave V3 technical paper(section 4.6), has a detailed explanation of the damages this issue could cause. Quoting the third part of the referenced section:

For the Aave Protocol, and other systems using oracle price-feeds, this means that the feeds are not updated while the sequencer is down (as they use transactions after all). Essentially having a case where the entire price-development that occurred throughout the downtime is applied when the sequencer comes up. This uncertainty, and possibility for ”slow flash crashes”, together with the fact that queuing L2 transactions directly at L1 is out of reach for most normal users lead Aave V3 to introduce a grace-period on liquidations in these exact cases. As long as the position is not heavily undercollateralized (0.95 < HF< 1), it will have grace period starting at the time the sequencer comes up until it can be liquidated. If the position goes below 0.95 it can be liquidated entirely as on L1. Note, that this grace period is only activated if the sequencer has been down. During the grace period users will also not be allowed to borrow.

[Bauchibred]

First I want to appreciate all what the CodeHawks team have been doing for the web3 security space, but I'd really like a second look at this issue since I believe this has wrongly being tagged as a duplicate to the issue of checking if the sequencer is up or not, where as they are somewhat related, I argue that this should be a stand alone issue, reasons below:

• This issue's case is rooted to when the sequencer just comes back up and users are being abruptly liquidated
• Do note that the recommended mitigation steps for the normal oracle sequencer down issue, is to input the check, which doesn't solve this issue, why?
• The answer to the question above is cause when the sequencer just comes back up the sequencer down check wouldn't fail and this case still exists, which means that users could get abruptly liquidated
• This is why the recommended mitigation steps as stated in my report is to instead have a grace period after the sequencer comes back up to allow users enough time to react.

[PatrickAlphaC]

To me it seems these are the same issues, but with slight different focuses and recommendations. Leaving as duplicates