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

Offline sequencer provides stale prices

0xBinChook

==== Github links ====
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L207-L207
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L218-L218

Summary

On L2 chains (including Arbitum) the Chainlink Oracles require a check for if the sequencer is down, avoiding prices from appearing fresh when they are not.

The bug could be leveraged by malicious actors to take advantage of the sequencer downtime.

Vulnerability Details

As outlined in the Chainlink L2 sequencer feeds, the Chainlink data feeds come from
Ethereum mainent (L1) and are propagated to their L2 counterparts.
If the Arbitum sequencer is down, the updates from L1 to L2 will not be occuring.

The only place in scope that uses Chainlink price feeds is the LiquidationPool.distributedAssets, where there is no check for the sequencer being down.

LiquidationPool::distributeAssets()

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
(,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
uint256 _portion = asset.amount * _positionStake / stakeTotal;
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}

Impact

LiquidationPool::distributedAssets() is called by LiquidationPoolManager.runLiquidation() as an essential part of the liquidation process.

The liquidation process is the secondary pegging mechanism outlined in TheStandard whitepaper

The pertinent scenario being the sequencer is down and there is significant price movement that triggers an update to the L1 price feed, but the update does not reach the L2 price feed (as the tx remains awaiting in the inbox).

Two possibilities are either over or under valuation of assets in LiquidationPool::distributedAssets().

  • Asset overvaluation: stakers looses out, paying more EUROs to the protocol for the assets.

  • Asset undervaluation: protocol looses out, taking fewer EUROs than it should from the stakers for the assets.

In the overall liquidation process, incorrect valuation can result in vaults being liquidated when should not be (based on L1 prices).

Tools Used

Manual review

Recommendations

Implement a check for the sequencer being up before retrieving the Chainlink price feeds.

Inspiration from the Chainlink example code and attempting the code style used by TheStandard.

Add the following into LiquidationPool

address private sequencerUptimeFeed;
​
uint256 private constant GRACE_PERIOD_TIME = 3600;
​
/// @dev Checks that the sequencer is up, reverting when it is not
function sequencerUpCheck() private {
(
/*uint80 roundID*/,
int256 answer,
uint256 startedAt,
/*uint256 updatedAt*/,
/*uint80 answeredInRound*/
) = Chainlink.AggregatorV3Interface(sequencerUptimeFeed).latestRoundData();
​
// Answer == 0: Sequencer is up
// Answer == 1: Sequencer is down
require(answer == 0, "sequencer-down");
​
// Ensure the grace period has passed after the sequencer is back up.
uint256 timeSinceUp = block.timestamp - startedAt;
require(timeSinceUp > GRACE_PERIOD_TIME, "sequencer-grace-period");
}

Then to actually apply the check, update LiquidationPool::distributeAssets()

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
+ sequencerUpCheck();
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();

The configuration value for Arbitum mainnet from Chainlink L2 sequencer uptime feeds.
Requires setting as the sequencerUptimeFeed, by either by an addition access controlled setter, or via the constructor.

Arbitrum mainnet: 0xFdB631F5EE196F0ed6FAa767959853A9F217697D
Updates

Lead Judging Commences

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

Arbitrum-sequncer

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

Arbitrum-sequncer

Support

FAQs

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