The Standard

Summary

If too many pendingStakes are submitted at the same time, then gas cost to enter the LP will increase quadratically for more stakers.

Similar to https://www.codehawks.com/finding/clossz9gy001syju0qkwn9g62

Vulnerability Details

Users can increase their position and stake using increasePosition in https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L134. It calls consolidatePendingStakes(). At the same time users can decrease their position and unstake in https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L149

The function consolidatePendingStakes() in https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L119
runs in O(n^2) time, this is because there is a for loop to iterate through all pendingStakes in O(n) time and if deletePendingStakes is called there is also another for loop which goes through every pendingStake and update its index in O(n) time. Therefore, there is a potential for stake consolidation to be done in O(n^2) time which result in gas cost that grows quadratically. This worst-case scenario can be easily reached and is outlined as follows:

1. Attacker adds a new pendingStake N times in block B to block B+k and adding a single pending stake runs at most O(N) time withing the block gas limit (as deletePendingStake is not called.)

2. After 1 day, assuming c blocks have passed, someone adds a new pendingStake block B+k+c.

3. Assuming no one added a pendingStake and thus consolidated their stakes from the blocks B+c to B+k+c-1, then there will still be N pendingStakes and in block B+k+c the deadline is reached for all N pendingStakes, so these N pending stakes has to be deleted which takes O(N) time per stake

4. It will take O(N^2) time to consolidate all pending stakes which can cause gas required to exceed block gas limit.

Impact

If the gas cost grows too high and exceeds block gas limit, then this two functions increasePosition and decreasePosition will be DoSed. This is results in no one being able to enter or exit the LP leading to loss of funds.

Tools Used

Manual Review

Recommendations

Use a mapping for pendingStakes instead.