sherlock-audit / 2024-02-smilee-finance-judging

2 stars 1 forks source link

y0ng0p3 - Not checking for Arbitrum L2 Sequencer #133

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 9 months ago

y0ng0p3

medium

Not checking for Arbitrum L2 Sequencer

Summary

Using Chainlink in L2 chains such as Arbitrum, smart contracts must check whether the L2 Sequencer is down to avoid stale pricing data that appears fresh.
The sequencer downtime can be leveraged by malicious actors to take advantage of the incorrect price reporting as well as it can lead to unfair liquidations due to stale price feeds.

Vulnerability Detail

The smart contracts will be deployed on any EVM chain which also includes L2 like Arbitrum.
The issue here is, if the Arbitrum Sequencer goes down, oracle data will not be kept up to date, and thus could become stale. However, users are still able to continue to interact with the protocol directly through the L1 Arbitrum rollup contract. You can review Chainlink docs on L2 Sequencer Uptime Feeds for more details on this.
Arbitrum rollup protocols move all execution off the layer 1 (L1) Ethereum chain, complete execution on a layer 2 (L2) chain, and return the result of the L2 execution back to the L1. These protocols have a sequencer that executes and rolls up the L2 transactions by batching multiple transactions into a single transaction.

If a sequencer becomes unavailable, it is impossible to access read/write APIs that consumers are using and applications on the L2 network will be down for most users not interacting directly through the L1 Arbitrum rollup contracts. The L2 has not stopped, but it would be unfair to continue providing service on your applications when only a few users can use them.

However, this check for active L2 Sequencer is missing in current implementation which can be seen here. As a result, users may be able to use the protocol while oracle feeds are stale. This could cause many problems, but as a simple example:

1- A user has an account with 100 tokens, valued at 1 ETH each, and no borrows

2- The Arbitrum sequencer goes down temporarily

3- While it’s down, the price of the token falls to 0.5 ETH each

4- The current value of the user’s account is 50 ETH, so they should be able to borrow a maximum of 200 ETH to keep account healthy ((200 + 50) / 200 = 1.2) Because of the stale price, the protocol lets them borrow 400 ETH ((400 + 100) / 400 = 1.2)

Impact

If the Arbitrum sequencer goes down, the protocol will allow users to continue to operate at the previous (stale) rates. A down L2 sequencer can result in :

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/chainlink/ChainlinkPriceOracle.sol#L110-L125

Tool used

Manual review

Recommendation

Refer to the Chainlink documentation for proper implementation of L2 Sequencer uptime checks through feeds. As this Chainlink's example, check if the L2 sequencer is up before fetching the price.

sherlock-admin3 commented 8 months ago

3 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, sequencer issues are invalid in sherlock, besides the underlying DEX price will prevent the usage of outdated oracle price

tsvetanovv commented:

According to Smilee Readme and Sherlock documentation this issue type is invalid

takarez commented:

invalid