The Standard

Summary

When a staker would withdraw full amount of both their position balance (positions[_stake.holder].TST and positions[_stake.holder].EUROs), their pending staked-balance of $TST and $EUROs are supposed to be also accounted.
Because if the both staked-balance would be 0 (positions[_stake.holder].TST == 0 && positions[_stake.holder].EUROs == 0) when the staker would withdraw full staked-balance of both via the LiquidationPool#decreasePosition(), the staker's position (positions[_stake.holder]) would be deleted - even if some pending staked-balance would be remained in the pendingStake array storage.

However, there is no validation to check whether or not the pending stake data (PendingStake) would be remained in the pendingStake array storage - when the LiquidationPool#decreasePosition() would be called.

This lead to losing the staker's pending stake balance.

Vulnerability Details

When a staker would like to decrease their staked-amount of $TST or/and $EUROs, the staker would call the LiquidationPool#decreasePosition().

Within the LiquidationPool#decreasePosition(), the LiquidationPool#consolidatePendingStakes() would be called.
And then, if the both staked-EUROs balance would be the empty balance (_position.TST == 0 && _position.EUROs == 0), the caller's position (positions[msg.sender]) would be deleted via the LiquidationPool#deletePosition() like this:
(NOTE：The empty balance check would be done via the LiquidationPool#empty())
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L150
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L161

Within the LiquidationPool#consolidatePendingStakes(), if this function would be called after the deadline (_stake.createdAt < deadline), the given pending stake data (PendingStake) would be consolidated to the given staker's staked-balance of $TST and $EUROs (positions[_stake.holder].TST and positions[_stake.holder].EUROs) like this:
(NOTE：Since the deadline would be defined as block.timestamp - 1 days, the pending stake period would be for 1 day after a staker staked $TST or/and $EUROs via the LiquidationPool#increasePosition())
https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L120-L127

When a staker would withdraw full amount of both their position balance (positions[_stake.holder].TST and positions[_stake.holder].EUROs), their pending staked-balance of $TST and $EUROs are supposed to be also accounted.
Because if the both staked-balance would be 0 (positions[_stake.holder].TST == 0 && positions[_stake.holder].EUROs == 0) when the staker would withdraw full staked-balance of both via the LiquidationPool#decreasePosition(), the staker's position (positions[_stake.holder]) would be deleted - even if some pending staked-balance would be remained in the pendingStake array storage.

However, there is no validation to check whether or not the pending stake data (PendingStake) would be remained in the pendingStake array storage - when the LiquidationPool#decreasePosition() would be called.

This lead to losing the staker's pending stake balance.

PoC

Assumption:

• Alice is a staker of $TST and $EUROs in the LiquidationPool.

Scenario:

• 1/ If Alice, who still has the pending stake balance in the pendingStakes array storage, would withdraw full amount of both their position's balance (positions[_stake.holder].TST and positions[_stake.holder].EUROs) by calling the LiquidationPool#decreasePosition() with the full amount of both their position's balance (positions[_stake.holder].TST and positions[_stake.holder].EUROs) as the _tstVal and the _eurosVal, the both balance (positions[_stake.holder].TST and the positions[_stake.holder].EUROs) would be 0 (empty balance) .
  And therefore, her position (positions[_stake.holder]) would be deleted - even if some pending staked-balance would be remained in the pendingStake array storage.

• 2/ 1 day later, the Alice's pending period would be end and her pending stake balance would be attempted to be consolidated to her position when someone would call the LiquidationPool#consolidatePendingStakes() via the following functions.

  • LiquidationPool#increasePosition()

  • LiquidationPool#decreasePosition()

  • LiquidationPoolManager#runLiquidation()

However, at that time, there is no target position for Alice's pending stake to be added (consolidated) - because their position would already be deleted when the step 1/ above.
As a result, her pending stake balance would be lost.

Impact

The staker's pending stake balance would be lost.

Tools Used

• Manual review

Recommendations

Within the LiquidationPool#empty(), consider adding a validation to check whether or not the pending stake (PendingStake) would be remained in the pendingStake array storage like this: