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

Unbounded loop in runLiquidation() can cause DOS

0xaraj

Summary

LiquidationPoolManager::runLiquidation can cause DOS as it has unbounded loop on holders length in distributeAsset function

Vulnerability Details

When liquidating a vault, runLiquidation is called which calls distributeAsset in LiquidationPool, which has an unbounded loop over holders length. As the number of holders increases gas cost will also increase, which eventually gets out of block gas limit

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
// 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();
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;
}
// code........
}

Read this for more https://github.com/wissalHaji/solidity-coding-advices/blob/master/best-practices/be-careful-with-loops.md

Impact

As this happens, undercollateralized vaults will be not liquidated

Tools Used

Manual Review

Recommendations

Possible option is include the loop functionality in clam reward function, where portion of staker will be calculated at the time of withdrawal

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

informational/invalid

Support

FAQs

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

Give us feedback!