The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

`distributeAssets` does not call `distributeFees` and therefore calling it directly will lead to an unfair distribution of assets

cosine

Summary

The runLiquidation function inside the LiquidationPoolManager contract is used to liquidate a vault and distribute all the collateral assets among the stakers inside the LiquidationPool contract by calling distributeAssets. The stakers do not get the collateral tokens for free, they need to buy them (for a discount) with EURO tokens.

The distributeFees distributes fees in EURO tokens among all stakers. Therefore, the runLiquidation function calls distributeFees before distributing the collateral tokens with distributeAssets as the aquired EURO tokens from the fees can influence how many collateral tokens each staker gets.

However, the distributeAssets function is an external function and can be called directly by anyone. If someone calls distributeAssets directly, the function will not call distributeFees as only runLiquidation does, and therefore the assets are most likely distributed in an unfair split, as the fees were not acquired.

Vulnerability Details

All the functions inside the LiquidationPool contract call consolidatePendingStakes(); and ILiquidationPoolManager(manager).distributeFees(); before interacting with any staking position to update the position to the current state. Except from distributeAssets which only calls consolidatePendingStakes(); and therefore does not distribute the fees before distributing the assets.

The reason for that is that distributeAssets is normally called by runLiquidation which already calls distributeFees before calling distributeAssets. However, distributeAssets is an external function and can be called by anyone. If someone calls distributeAssets directly, the function will not call distributeFees and therefore the assets are most likely distributed in an unfair split, as the fees were not acquired.

Here we can see that distributeAssets can be called by anyone and that it does not call distributeFees to update the current EURO value of the staking positions:

// LiquidationPoolManager
function distributeFees() public {
IERC20 eurosToken = IERC20(EUROs);
uint256 _feesForPool = eurosToken.balanceOf(address(this)) * poolFeePercentage / HUNDRED_PC;
if (_feesForPool > 0) {
eurosToken.approve(pool, _feesForPool);
LiquidationPool(pool).distributeFees(_feesForPool);
}
eurosToken.transfer(protocol, eurosToken.balanceOf(address(this)));
}
​
// LiquidationPool
function distributeFees(uint256 _amount) external onlyManager {
uint256 tstTotal = getTstTotal();
if (tstTotal > 0) {
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
for (uint256 i = 0; i < holders.length; i++) {
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;
}
}
}
​
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

Unfair distribution of assets among stakers.

Recommendations

The distributeAssets function should be restricted to only be callable by the LiquidationPoolManager contract.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

distributeAssets-issue

Support

FAQs

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

Give us feedback!