sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

nobody2018 - Missing Sequencer Uptime Feed check can cause unfair liquidations #164

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 7 months ago

nobody2018

medium

Missing Sequencer Uptime Feed check can cause unfair liquidations

Summary

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

This leaves long-party no time to react to the price changes which can lead to unfair liquidations.

Vulnerability Detail

Chainlink explains their Sequencer Uptime Feeds in [docs](https://docs.chain.link/data-feeds/l2-sequencer-feeds).

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.

This issue lies in _getOnchainPrice(), which do not use the sequencer uptime feed to check the status of the sequencer:

File: flatcoin-v1\src\OracleModule.sol
141:     function _getOnchainPrice() internal view returns (uint256 price, uint256 timestamp) {
142:         IChainlinkAggregatorV3 oracle = onchainOracle.oracleContract;
143:         if (address(oracle) == address(0)) revert FlatcoinErrors.ZeroAddress("oracle");
144:         
145:         (, int256 _price, , uint256 updatedAt, ) = oracle.latestRoundData();
146:         timestamp = updatedAt;
147:         // check Chainlink oracle price updated within `maxAge` time.
148:         if (block.timestamp > timestamp + onchainOracle.maxAge)
149:             revert FlatcoinErrors.PriceStale(FlatcoinErrors.PriceSource.OnChain);
150: 
151:         if (_price > 0) {
152:             price = uint256(_price) * (10 ** 10); // convert Chainlink oracle decimals 8 -> 18
153:         } else {
154:             // Issue with onchain oracle indicates a serious problem
155:             revert FlatcoinErrors.PriceInvalid(FlatcoinErrors.PriceSource.OnChain);
156:         }
157:     }

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

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L141-L157

Tool used

Manual Review

Recommendation

The Chainlink documentation contains [an example](https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code) for how to check the sequencer status and shows [available network](https://docs.chain.link/data-feeds/l2-sequencer-feeds#available-networks) including Base.

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

Duplicate of #27

sherlock-admin commented 6 months ago

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

takarez commented:

invalid

securitygrid commented 6 months ago

Escalate This issue is not dup of #27. This issue and #42 are same. #27 and other dups are same. This report describles:

When the Base sequencer is down and then comes back up, all Chainlink price updates will become available within a very short time. This leaves long-party no time to react to the price changes which can lead to unfair liquidations.

sherlock-admin2 commented 6 months ago

Escalate This issue is not dup of #27. This issue and #42 are same. #27 and other dups are same. This report describles:

When the Base sequencer is down and then comes back up, all Chainlink price updates will become available within a very short time. This leaves long-party no time to react to the price changes which can lead to unfair liquidations.

You've created a valid escalation!

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.

nevillehuang commented 6 months ago

No additional comments, this should remain invalid based on comments here, so correct duplication doesn't matter given they share the same root cause of L2 Sequencer being down.

Evert0x commented 6 months ago

Agree with lead judge, planning to invalidate

Evert0x commented 6 months ago

Result: Invalid Duplicate of #27

sherlock-admin2 commented 6 months ago

Escalations have been resolved successfully!

Escalation status: