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

`LiquidationPool::distributeAssets` may use stale Chainlink price due to missing check the validity of returned data or inactive Arbitrum Sequencer

bube

Summary

In the LiquidationPool::distributeAssets function is used Chainlink latestRounData function to retrieve the EUR/USD and Token/USD prices. But the returned prices can be incorrect or stale. Therefore, it is important to check the returned values from Chainlink before their use. Also, the contract will be deployed on Arbitrum. When utilizing Chainlink in L2 chains like Arbitrum, it's important to ensure that the prices provided are not falsely perceived as fresh particularly in scenarios where the sequencer might be non-operational. Therefore, it is required to confirm the active status of the sequencer before trusting the data returned by the oracle.

Vulnerability Details

The LiquidationPool::distributeAssets function relies on Chainlink latestRoundData function to retrieve the EUR/USD and Token/USD prices. But the function does not check if the returned price from Chainlink is not stale.

​
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();
.
.
.
}
}
}
}

That can lead to issues if Chainlink starts a new round and struggles to establish consensus on the new value for the oracle. Without proper checks, the function LiquidationPool::distributeAssets may continue using outdated, stale, or incorrect price if oracles are unable to submit and start a new round. Possible reasons for this could include Chainlink nodes abandoning the oracle, chain congestion, or vulnerabilities/attacks on the Chainlink system.

Additionally, there is no check if the Arbitrum Sequencer is active. In the event of an Arbitrum Sequencer outage, the oracle data may become outdated, potentially leading to staleness. You can review Chainlink docs on L2 Sequencer Uptime Feeds for more details on this.
https://docs.chain.link/data-feeds/l2-sequencer-feeds

Impact

In the LiquidationPool::distributeAssets function, asset distribution is based on the current price of the EUROs and the assets being distributed. Stale prices could lead to incorrect distribution of assets, with users receiving either too much or too little compared to the actual market value. The function also calculates the cost in EUROs for the portion of assets distributed to each holder. Stale prices could lead to rewards being priced incorrectly, affecting the amount of EUROs burned and the rewards given to users.

Also, in the scenario where the Arbitrum Sequencer experiences an outage, the protocol will enable users to maintain their operations based on the previous (stale) rates.

Tools Used

Manual Review

Recommendations

To mitigate this issue, it is recommended to implement checks to ensure that the price returned by Chainlink is not stale. You can add the following code to validate the price obtained from Chainlink:

​
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
- (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ ( roundId, priceEurUsd, , updateTime, answeredInRound ) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ require(priceEurUsd > 0, "Chainlink price <= 0");
+ require(updateTime != 0, "Incomplete round");
+ require(answeredInRound >= roundId, "Stale price");
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();
+ ( roundId, assetPriceUsd, , updateTime, answeredInRound ) = Chainlink.AggregatorV3Interface(asset.token.clAddr). latestRoundData();
+ require(assetPriceUsd > 0, "Chainlink price <= 0");
+ require(updateTime != 0, "Incomplete round");
+ require(answeredInRound >= roundId, "Stale price");
.
.
.
}
}
}
}

To check if the Arbitrum Sequencer is alive, you can add the following function and use it in require statement in the distributeAssets function:
Here is a code example on Chainlink docs: https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

function isSequencerAlive() internal view returns (bool) {
(, int256 answer, uint256 startedAt,,) = sequencer.latestRoundData();
if (block.timestamp - startedAt <= GRACE_PERIOD_TIME || answer == 1)
return false;
return true;
}
Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Arbitrum-sequncer

hrishibhat Lead Judge almost 2 years 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.

Give us feedback!