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

Missing access control in `distributeAssets` function

0xaraj

Summary

LiquidationPool::distributeAssets can be called by any malicious user because of missing access control which will cause LiquidationPool to loss funds

Vulnerability Details

A user is liquidated through runLiquidation function in LiquidationPoolManager, which calculates which token user was holding along with amount, and calls the distributeAssets function in LiquidationPool with assets, collateralRate, HUNDRED_PC as parameters, but distributeAssets is missing access control that onlyManager should call this function

function runLiquidation(uint256 _tokenId) external {
// code......
@> LiquidationPool(pool).distributeAssets{value: ethBalance}(assets, manager.collateralRate(), manager.HUNDRED_PC());
forwardRemainingRewards(tokens);
}

Here we can see distributeAssets has no access control

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

How this will work (POC)

  1. User will call this distributeAssets with malicious parameters

  2. distributeAssets is calculating _portion based on _assets.amount, this will be inflated to get more portion

uint256 _portion = (asset.amount * _positionStake) / stakeTotal;

  1. costInEuros of that portion can be reduced because it is calculated based on _collateralRate & _hundredPC

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

  1. EURO from position will be reduced and reward will be set

  2. Now, functions is transferring token from manager to LiquidationPool address, this will revert if manager has not enough tokens but there is no check for ETH which means if we pass only ETH in _asset parameter then it will work because its only increasing nativePurchased( ie will set reward and will take less euro for that )

if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}

Impact

Liquidation pool will loss funds as wrong rewards has been set by malicious user

Tools Used

Manual Review

Recommendations

Use onlyManager access control in distributeAssets like we've done in distributeFees

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!