The Standard

Summary

The function claimRewards() has no reentrancy protection and could be used repeatedly in an exploit to drain user funds in the protocol.

Vulnerability Details

In order to exploit this vulnerability a malicious user would have to stake into the protocol with a contract created and deployed by them with a fallback function that will

1. Be triggered by

(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");

in claimRewards().

2. Re-enter the LiquidationPool.sol and call the distributeAssets() function as described below.

3. After the previous call, calls claimRewards() Again.

The input parameters for calling distributeAssets are as follows:

LiquidationPoolManager.Asset[] memory _assets

the _assets[index].amount would have to be greater than zero, then the _asset[index].token.dec, _hundredPC and _collateralRate input parameters will have to be modified in such a way that the user can gain a maximum _portion. This is to gain maximum possible rewards:

rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion

However, the complexity arises in that the total asset amount has to be equal to the native purchased variable which is the sum of all the portions in order to bypass the line

(bool _sent,) = manager.call{value: _assets[index].amount - _nativePurchased}("");

The best condition for an attacker in this case is if all holders have staked euro token that is greater than zero then they could set the _asset[index].token.dec value to 18 in order to ensure

uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;

costInEuros equates to zero and this will allow the asset amount to be equal to all the portions staked. However if this is not the case, then modifying the costInEuros variable to get an equilibrium between _assets[index].amount and nativePurchased is highly unlikely to be efficient due to third party oracles but theroretically possible.

The last possible but highly likely situation is if there are already unclaimed native assets in the protocol this could make this much easier for an attacker.

The other requirement is

_assets[index].token.addr

will have to be equal to address(0) to bypass token transfers and

_assets[index].token.symbol

will have to be equal to Native token symbol.

The contract will also be required to have an external function to call claimRewards().

Impact

This is attack is more effective and efficient in draining protocol funds and the root cause is the lack of reentrancy protection, in that other users will have an insolvent protocol in a single transaction.

Tools Used

Manual Review

Recommendations

I would recommend a reentrancy guard be added to the claimRewards function, the oppenzeppilin standard is highly recommended.