No check for sequencer uptime can lead to dutch auctions failing or executing at bad prices

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/main/src/rate-limits/RateLimitedMinter.sol#L52

Vulnerability details

Impact

The AuctionHouse contract implements a Dutch auction mechanism to recover debt from collateral. However, there is no check for sequencer uptime, which could lead to auctions failing or executing at unfavorable prices.

The current deployment parameters allow auctions to succeed without a loss to the protocol for a duration of 10m 50s. If there's no bid on the auction after this period, the protocol has no other option but to take a loss or forgive the loan. This could have serious consequences in the event of a network outage, as any loss results in the slashing of all users with weight on the term.

Network outages and large reorgs happen with relative frequency. For instance, Arbitrum suffered an hour-long outage just two weeks ago (source).

Proof of Concept

Consider the following scenario:

1. A loan is called and an auction is initiated.
2. The network experiences an outage, causing the sequencer to go offline.
3. The auction fails to receive any bids within the 10m 50s window due to the outage.
4. The protocol is forced to take a loss (if there's still a bid after the midPoint and before the auction ends) or forgive the loan, both leading to the complete slashing of all users with weight on the term.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue, consider integrating an external uptime feed such as Chainlink's L2 Sequencer Feeds. This would allow the contract to invalidate an auction if the sequencer was ever offline during its duration. Alternatively, implement a mechanism to restart an auction if it has received no bids.

Assessed type

Other

[c4-pre-sort]

0xSorryNotSorry marked the issue as sufficient quality report

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[eswak]

Acknowledging this, we definitely don't want to add a dependency on an oracle to run the liquidations, so maybe another fix would be to define auction durations in number of blocks, but I think basing auctions on time is semantically correct because it depends on market conditions (that are based on time) and when the sequencer resumes the conditions that triggered the auction might not hold anymore. The forgive() path at the end of the auction could be used to unstuck the situation at the smart contract level, and the governance can organize an orderly fix of the situation when the sequencer resumes. Given the likelihood I think it should be low severity, especially since we know auctions have to be longer on L2s than on mainnet (liquidity potentially needs to bridge, etc), so the chance that a downtime large enough relative to the auction duration will happen is pretty low.

[c4-sponsor]

eswak (sponsor) acknowledged

[c4-sponsor]

eswak marked the issue as disagree with severity

[Trumpero]

Imo, if an auction is still active when the sequencer is down, it may cause a loss of assets (collateral) for the borrower. Although governance's measures can help mitigate the situation when the sequencer resumes, loss and slashing in this lending term will be inevitable in such cases. So I think medium severity is appropriate for this issue, but I'm open to other feedback.

[c4-judge]

Trumpero marked the issue as satisfactory

[c4-judge]

Trumpero marked the issue as selected for report