The Standard

Summary

Due to the time delay between the call of LiquidationPool.increasePosition() and the processing of the user's pending stake, a user's position may be deleted in between these two operations. This results in the stake being consolidated into the user's position without the user's address being present in the holders array. Consequently, this user will go unnoticed by most methods in the LiquidationPool, leading to underestimated total staked volumes and the user's inability to receive rewards from distributeFees()/distributeAssets().

Vulnerability Details

The current process of increasing user positions involves the following steps. First, Alice calls increasePosition(), after which she is added to the holders array, and her stake is included in pendingStakes with a 24-hour deadline. Any subsequent calls to increasePosition(), decreasePosition(), or distributeAssets() after 24 hours triggers consolidatePendingStakes(), consuming Alice's pending stake and updating her position struct.

When consolidatePendingStakes() processes pending stakes, it implicitly assumes that the pending stakers is in the holders array, which is not always true and can lead to problems as detailed below. Note that there is a mismatch between when the user address is added to the holders array (during increasePosition()) and when the position struct is updated (during consolidatePendingStakes()). Consequently, the position can be deleted between these two actions, for example, by calling decreasePosition(). This results in the position struct being updated correctly, but the user's address won't be present in the holders array.

Consider the following sequence of events for Alice, who has a initial position of 100 EURO and 100 TST:

1. At t = 0, Alice calls increasePosition(20, 20).

2. At t = 2h, Alice wants to reduce her initial position by calling decreasePosition(100, 100). After the reduction, her EURO/TST balances are zero, and her position is deleted, removing her address from the holders array. However, Alice's pending stake is still waiting to be processed due to the 24-hour deadline.

3. At t > 24h, a user calls increasePosition()/decreasePosition(), triggering the call to consolidatePendingStakes(). Consequently, Alice's pending stake is processed and added to her position struct, but her address is not present in the holders array.

Unbeknownst to Alice, she is not accounted among the holders, rendering her stake useless. Many crucial functions in LiquidationPool, such as distributeFees() or distributeAssets(), iterate over the holders array to process holder's position balances and distribute proportional rewards. However, as Alice's address is not in the holders array, she cannot be accounted for in total staked amount calculations, nor will she receive rewards for her stake.

Impact

Affected users will be unaccounted for by most methods in the LiquidationPool, resulting in underestimated total staked volumes and an inability to receive rewards from distributeFees()/distributeAssets().

Tools Used

Manual Review.

Recommended Mitigation

Consider moving the LiquidationPool.addUniqueHolder() call from increasePosition() to the consolidatePendingStakes() function.