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

Staked position receives no fees or rewards

sec305842

Summary

If a user withdraws all tokens from a previous position while they have a pending stake, their pending stake will never receive any rewards, nor fees once it gets consolidated into a position.

Vulnerability Details

  • user has an active position in LiquidationPool

  • user calls increasePosition to add a pending stake

  • user calls decreasePosition to withdraw all tokens from their position while their other stake is still pending

  • decreasePosition calls deletePosition which calls deleteHolder, the user is removed from the holders array

  • when consolidatePendingStakes gets called after 1day, the pending stake is consolidated into a position with no matching entry in the holders array

  • fee and reward distribution iterate over the holders array so this position will never receive anything

describe('increase-increase-decrease-consolidate position', async () => {
it('properly consolidates pending positions after decrease', async () => {
// stake 1 tst - pending
const tstStake1 = ethers.utils.parseEther('1');
await TST.mint(user1.address, tstStake1);
await TST.approve(LiquidationPool.address, tstStake1);
await LiquidationPool.increasePosition(tstStake1, 0);
​
await fastForward(DAY);
​
// stake 2 tst - previous pending 1 tst is consolidated
const tstStake2 = ethers.utils.parseEther('2');
await TST.mint(user1.address, tstStake2);
await TST.approve(LiquidationPool.address, tstStake2);
await LiquidationPool.increasePosition(tstStake2, 0);
​
// withdraw stake 1
await LiquidationPool.decreasePosition(tstStake1, 0);
​
await fastForward(DAY);
​
// generate some fees - 10 EUROs
const fees = ethers.utils.parseEther('10');
await EUROs.mint(LiquidationPoolManager.address, fees);
​
// consolidate stake 2, distribute fees and withdraw all (2 TST, 5 EUROs)
const distributedFees = ethers.utils.parseEther('5');
await LiquidationPool.decreasePosition(tstStake2, distributedFees);
expect(await TST.balanceOf(user1.address)).to.equal(tstStake1.add(tstStake2));
expect(await EUROs.balanceOf(user1.address)).to.equal(distributedFees);
});
});

Impact

No known attack vector. It is simply a bug and inconvenience for the users. The user will not receive fees or rewards for their stake but they can still withdraw by calling decreasePosition or fix the position by adding more stake via increasePosition.
However, such inconsistencies in state could easily lead to vulnerabilities down the road.

Recommendations

Do not remove user from holders array if they have a pending stake or re-add them when the pending stake is consolidated into a position.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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