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

Lack of Price Feed validation can cause reverts or incorrect asset valuations

falconhoof

Vulnerability Details

In LiquidationPool::distributeAssets() two Chainlink price feeds are used to return the latest price information for the assets being liquidated so that they can be distributed to eligible holders.

The issue arises from the lack of checks on the returned prices. They are accepted without any validation which is risky as incorrect / stale data can be returned. The data should be validated before being used as Chainlink may not be reliable where it struggles to establish a consensus from among it's sources at the beginning of a new round; where chain congestion or attacks on Chainlink means oracles are unable to submit and start a new round.
A further check is recommended if deploying the Arbitrum chain where Chainlink recommends a check that the Arbitrum Sequencer is live via the Sequencer Uptime Feed.

Impact

If an incorrect value of 0 were returned by either of the chainlink price feeds; this would revert the entire liquidation transaction flow.
The sponsor acknowledges the distributeAssets() function's dependency on non-zero price feed values in the audit's README.md file.

If stale values are returned, i.e. an old value, it will result in incorrect asset valuations, i.e. in calculating the costInEuros variable, in distributeAssets(). An overvaluation or undervaluation means holders can be sold assets at a lower or higher discount rate than the 9.09% that it should be.
This breaks the functionality of the protocol as holders will lose a portion of their financial incentive for staking assets if assets are overvalued or they may receive undue rewards where assets are undervalued. This in turn affects the equilibrium between EUROs and collateral as if assets are overvalued, more EUROs are required to buy collateral.

Tools Used

Manual Review

Recommendations

Add zero value, stale value and roundId checks as below. For Arbitrum Sequencer check the linked code can be used https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
- (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ (uint80 roundId, int256 priceEurUsd,, uint256 updatedAt, uint80 answeredInRound) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ require(priceEurUsd > 0, "Chainlink price <= 0");
+ require(updateTime != 0, "Incomplete round");
+ require(answeredInRound >= roundId, "Stale price");
// Some Code
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();
+ (uint80 roundId, int256 assetPriceUsd,, uint256 updatedAt, uint80 answeredInRound) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ require(assetPriceUsd > 0, "Chainlink price <= 0");
+ require(updateTime != 0, "Incomplete round");
+ require(answeredInRound >= roundId, "Stale price");
// Some Code
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}
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.