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

A DoS attacker can make liquidation of vaults never happen

nuthan2x

Summary

  • An atatcker can stuff the holders array to DoS the liquidation transaction , where many state changes are happening as shown below.

Vulnerability Details

  • The vulnerability lies in the state layout and design of Liquidation pool. We use array to store the holders addresses.

  • A DoS attacker can make the liquidations never happen due to the out of gas error. An attacker can fill the holders array with 1000+ entries. So now the LiquidationPool.distributeAssets has to run 1000 holders * 5 assets so around 5000 transferFrom state changes + another 5000+ reward state change + another 1000+ positions state changes.

  • see the arrow marks below for the state changes.

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

HIGH (leaving bad debts to Liquidation pool stakers)

Tools Used

Manual review

Recommendations

  • Redesign the LiquidationPool control, to not do these many state change, but just use ERC4626 vault architecture, where shares are being used to denote how many assets staked, and whenever the balance of the vault increases, the price of each share increases.

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!