The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`LiquidationPool::distributeAssets` lacks validation to ensure Arbitrum sequencer is not down

DarkTower

Summary

There is no validation to ensure that the Arbitrum sequence is up. Noting that the contract will be deployed on Arbitrum L2 (with current versions already deployed) with a dependency on Chainlink, it's important to ensure that prices returned are correct/accurate and assumed to be fresh data which will not be the case in scenarios where the Arbitrum sequencer is down or not operating.

Vulnerability Details

The specific lines of code with this issue are at Line:207 of the distributeAssets function where it fetches the price of eurUsd and also at Line:218 where it fetches the assetPriceUsd which is the returned USD price for the asset token.

FILE: LiquidationPool.sol: Line 207
@> (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
​
FILE: LiquidationPool.sol: Line 207
@> (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
​

Impact

In the event of the Arbitrum Sequencer experiencing an outage, the returned data will be outdated leading to stale data. Using Chainlink in Arbitrum requires checking if the sequencer is down to avoid prices from looking like they are fresh although they are not. In such a case where the sequencer is down, stale price is used for arriving at the asset costInEuros and the protocol could be forced to burn too many or too few EUROs tokens as well as potentially deducting fewer EUROs than intended from the position.

The bug could be leveraged by malicious actors to take advantage of the sequencer downtime or cause harm as the distributeAssets function is external and lacks access control.

Tools Used

Manual review

Recommendations

A step towards remediating this is to confirm that the status of the Arbitrum Sequencer is active before trusting/utilizing the returned data.
Chainlink has the functionality for checking Sequencer statuses which can be used for this verification purpose. Review Chainlink docs about this via: https://docs.chain.link/data-feeds/l2-sequencer-feeds

...
address private immutable eurUsd; // chainlink price feed for EURUSD
+ address private immutable sequencerUptimeFeed; // sequencer address
+ uint256 private constant GRACE_PERIOD_TIME = 3600; // grace period for Sequencer
...
consolidatePendingStakes();
+ (,int256 answer,uint256 startedAt,,) = Chainlink.AggregatorV3Interface(sequencerUptimeFeed).latestRoundData();
+ bool isSequencerUp = answer == 0;
+ if (!isSequencerUp) {
+ revert SequencerDown();
+ }
+ uint256 timeSinceUp = block.timestamp - startedAt;
+ if (timeSinceUp <= GRACE_PERIOD_TIME) {
+ revert GracePeriodNotOver();
+ }
​
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Arbitrum-sequncer

Chainlink-price

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Arbitrum-sequncer

Chainlink-price

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.