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

User rewards lost due to rounding down in `LiquidationPool.sol#distributeAssets()`

amarfares

Vulnerability Details

When a vault gets liquidated, the liquidated assets are distributed in LiquidationPool.sol#distributeAssets(). The way rewards are calculated for users in the distributeAssets() function is it loops through the holders.length, caches all the holders' _positionstake and then loops through the list of available liquidated assets to assign portions of to all the holders.

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L205-L241

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);
}

The problem here is that the _portion of rewards assigned to users is calculated in the following way:

uint256 _portion = asset.amount * _positionStake / stakeTotal

Where stakeTotal is the total combined amount of staked EUROs/TSTs for all holders in the protocol which could naturally be a huge amount. That large amount combined with a relatively small-medium liquidated vault asset.amount or/and a small enough user's _positionStake could mean that asset.amount * _positionStake < stakeTotal which would round down and return 0 rewards for that user.

This would lead to a loss of rewards for users and since the rewards are distributed proportionally, other users won't receive them either, but instead they will be forwarded back to the protocol owners since the last mechanism of liquidating a vault is forwarding any leftover rewards to the owners in LiquidationPoolManager.sol#runLiquidation():

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPoolManager.sol#L59-L82

LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);

Impact

Some user rewards can be lost when a vault is liquidated due to rounding down precision loss.

Tools Used

Manual Review

Recommendations

Revert if _portion == 0 or maybe skip rewards for such users and accrue it to the rest of the users` rewards?

Updates

Lead Judging Commences

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

precision

Support

FAQs

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

Give us feedback!