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

Chainlink prices are not checked for staleness

khramov

Summary

There is no check for stale prices in LiquidationPool::distributeAssets.

Vulnerability Details

Chainlink recommends checking that the latestTimestamp of the answer is recent enough:

Your application should track the latestTimestamp variable or use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for your application to use it. If your application detects that the reported answer is not updated within the heartbeat or within time limits that you determine are acceptable for your application, pause operation or switch to an alternate operation mode while identifying the cause of the delay.

Currently, it is not checked:
@> 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);
}

Possible scenario (unlikely but possible, just to illustrate the issue). The issue is also valid during liquidations:

  • distributeAssets could be called by anyone.

  • There is 1 BTC in the pool.

  • Current BTC price is 10_000 EUR/BTC.

  • Stale price in Chainlink is 20_000 EUR/BTC.

  • 20,000 EUR (not taking into account _collateralRate to simplify) would be got from holders and burnt.

  • Holders receive 1 BTC.

  • They sell 1 BTC.

  • They receive 10_000 EUR.

  • Holders' loss is 10_000 EUR.

Impact

Assets from LiquidationPool could be distributed to holders by non-market prices (higher than real prices), holders will lose some funds since they couldn't buy back the same amount of EUROs after selling distributed assets.

Tools Used

Manual review

Recommended Mitigation

  • Use PriceCalculator as a single source of prices.

  • In PriceCalculator, check that the price is not older than:

    • Heartbeat for stable coin collaterals.

    • N (for example, 1 hour) for volatile collaterals (BTC, ETH, etc).

Updates

Lead Judging Commences

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

Chainlink-price

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

Chainlink-price

Support

FAQs

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