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

LiquidationPool : anyone can call `distributeAssets` with any percentage value

ak1

Summary

Since the function distributeAssets can be called by anyone with the any percentage value. This would undermine the euro cost calculation which lead of deducting more or less portion EUROs

Vulnerability Details

distributeAssets can be called by anyone with uint256 _hundredPC

The purpose of this function is to distribute the assets to the user based on their staked shares. It is called from runLiquidation.

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

for (uint256 i = 0; i < tokens.length; i++) {
ITokenManager.Token memory token = tokens[i];
if (token.addr == address(0)) {
ethBalance = address(this).balance;
if (ethBalance > 0) assets[i] = ILiquidationPoolManager.Asset(token, ethBalance);
} else {
IERC20 ierc20 = IERC20(token.addr);
uint256 erc20balance = ierc20.balanceOf(address(this));
if (erc20balance > 0) {
assets[i] = ILiquidationPoolManager.Asset(token, erc20balance);
ierc20.approve(pool, erc20balance);
}
}
}
LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC()); ------>> refer here HUNDRED_PC
forwardRemainingRewards(tokens);

when we look at the distributeAssets. it is public and can be called by anyone.

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

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {

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

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; ---------------->>> _hundrePC is used here
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;

But, this function distributeAssets can be called by anyone with any _hundredPC more than 100 percent or less which would influence the costInEuros

Impact

costInEuros would be influenced in updating the _position.EUROs
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L218-L227

(,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;

Tools Used

Manual review.

Recommendations

we would suggest to add the access restriction for distributeAssets so that only the LiquidationPoolManager can call this.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year 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.