When a vault is liquidated the protocol distribute the assets of that vault to all the stakers as rewards(more like buying at discounted price).
This is done through the distributeAssets function in the LiquidationPool contract which updates the rewards mapping for the staker which he can later claim it.
Whenever a liquidation occurs it is called from the LiquidationPoolManager::runLiquidation which will liquidate the vault and send all the assets to the LiquidationPoolManager contract and then distribute it to the stakers by approving and calling the distributeAssets function.
When rewards are distributed it is not directly sent to the stakers but we map the amount with their address and the LiquidationPool contract still holds the rewards which the stakers can later claim it.
The problem is there is no access control in the distributAssets function:
this means anyone can call it with arbitrary values.
Example:
Say the LiquidationPool currently holds 10,000 eth in unclaimed rewards.
A malicious staker could call the distributeAssets function with the native currency as the asset to increase his rewards mapping and claim the rewards and repeat until he has drained all the rewards. Although he may need to burn some of his EUROs.
This will work only for the native currency because with the native currency the attacker doesn't have to send any value but with erc20 tokens the function will try to transfer form the manager contract which is the LiquidationPoolManager contract that might not have approved the LiquidationPool.
The function calls returnUnpurchasedNative(_assets, nativePurchased); but this will not be a problem for the attacker because if we look at the function it sends the native currency from the liquidationPool contract to the manager and the attacker doesn't have to spend any eth(in ethereum).
See:
Or
An attacker who is not even a staker can:
Call distributeAssets with the asset as the native currency and asset.amount as a very large number.
Increase the rewards for every staker by very large number.
And when users try to claim their rewards the claimRewards function will revert because the contract doesn't have enough native currency which will result in users not being able to claim even the other assets while his EUROs staked were also lost.
Impact
A staker can drain all the native currency held by the LiquidationPool contract as rewards and cause the claimRewards function to revert.
An attacker can increase the rewards count of the native asset for all stakers which the contract doesn't have which will DOS the claimRewards function and as a result the stakers lost EUROs and rewards of the all the asset tokens being unclaimable.
A staker can drain all the native currency held by the LiquidationPool contract as rewards and cause the claimRewards function to revert.
An attacker can increase the rewards count of the native asset for all stakers which the contract doesn't have which will DOS the claimRewards function and as a result the stakers lost EUROs and rewards of the all the asset tokens being unclaimable and lock all the assets in the contract.
Even if the users want to unstake here their position.EUROs already got decreased inside the distributeAssets function.
_position.EUROs -= costInEuros;
positions[holders[j]] = _position;
manual.
Add onlyManager modifier in the distributeAssets function.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.