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

Users participating in staking can get removed unintentionally from the `holders` array even if they still hold stake inside the `LiquidationPool` contract thus can't receive their rewards.

shikhar229169

Summary

The user's can get removed unintentionally from the holders array and will not receive their rewards generated from liquidation or from interest gained on their staked TST token.
If the user has staked their tokens inside LiquidationPool must be present in the holders array, but due to a mishandling inside the LiquidationPool::decreasePosition function, a user can get removed from the mentioned array.

Vulnerability Details

The vulnerability lies inside the LiquidationPool contract inside the decreasePosition function at line 161, where the user is removed from holders array when their position of both token is 0.
But consider the case where no doubt the user's position of both token became 0, but they still have position inside the pendingStakes and when their position is consolidated they are not added inside the holders array leading to isolating the stakers from earning rewards and EUROs gain on their staked TST.

Impact

Stakers will not be able to receive rewards and EUROs gain based on their staked TST as they were unintentionally removed from the holders array.
The likelihood of this happening is kind of medium as it may happen that user first staked some amount of EUROs and after 24 hrs they staked TST and removed their EUROs, but when they removed whole of their EUROs they are removed from the holders array but still they have position inside the pendingStake and again after 24 hrs when their TST will be consolidated they are not added to the holders array.

PoC

Add the below test inside test/liquidationPool.js

Run the test:

yarn hardhat test test/liquidationPool.js --grep "holders get deleted from holder array even though they have stake"
describe("Increasing & Decreasing Position", () => {
it("holders get deleted from holder array even though they have stake", async() => {
const euroAmount = ethers.utils.parseEther("1000")
await EUROs.mint(user1.address, euroAmount.mul(2))
​
// user1 approves pool for euros
await EUROs.connect(user1).approve(LiquidationPool.address, euroAmount)
​
// user1 increases their euro position in the pool
await LiquidationPool.connect(user1).increasePosition(0, euroAmount)
​
​
// then after 24 hrs they again increase their euros position
fastForward(DAY)
​
await EUROs.connect(user1).approve(LiquidationPool.address, euroAmount)
await LiquidationPool.connect(user1).increasePosition(0, euroAmount)
​
let holderAtZeroIdx = await LiquidationPool.holders(0)
expect(user1.address).to.equal(holderAtZeroIdx)
​
fastForward(DAY)
​
// And after 24hrs user again removes the euro amount
await LiquidationPool.connect(user1).decreasePosition(0, euroAmount)
​
// It is now expected that the user must have holding of euros
// equal to `euroAmount` and also their address should be present in holder array
const { _position } = await LiquidationPool.position(user1.address)
​
expect(_position.EUROs).to.equal(euroAmount)
await expect(LiquidationPool.holders(0)).to.be.reverted
})
})

Tools Used

Manual Review

Recommendations

1 Before deleting the user inside the decreasePosition function, consider checking if they have pending stake inside the pendingStakes array and if they have pending stake then don't delete them from holders array.

Alternatively,

2 Add the user by calling the addUniqueHolder function when a user's pending stake is consolidated.

Updates

Lead Judging Commences

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

deletePosition-issye

Support

FAQs

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