The Standard

Summary

The consolidatePendingStakes() function could be DOSed by anyone, locking all funds in the LiquidationPool contract.

Vulnerability Details

The consolidatePendingStakes() function is called in every main external function inside the LiquidationPool contract. In this function, it iterates through the pendingStakes list to find and delete elements using another for-loop. This means the function has a time complexity of O(n^2). This is not recommended for a smart contract as the gas cost will quickly grow as n increases and could potentially break the block gas limit.

When an attacker spams increasePosition() to add elements to the pendingStakes list, the amount of loop iteration they need for each call is only O(n) if all the calls are made within 24 hours. For example, if the attacker pushes n = 50000 elements to the list, they would need 1 + 2 + ... + 50000 = n*(n+1)/2 ~ n^2 iterations , which means they just need to spend the amount of gas equal to 1 block gas limit to DOS the whole contract.

Impact

When the number of pendingStakes is large enough to break the block gas limit, the consolidatePendingStakes() function will always revert. Since consolidatePendingStakes() is called in decreasePosition() and distributeAssets(), these functions will also always revert. For decreasePosition(), this means no one could unstake their funds. And for distributeAssets(), no vault could be liquidated.

Tools Used

Manual Review

Recommendations

Several improvements are necessary:

• Optimize consolidatePendingStakes() to O(n) time complexity. This could easily be done by finding the last element that should be deleted and then do only 1 delete operation.

• Limit the number of pendingStakes[]. This would mean increasePosition() could still be DOSed but it's better than freezing all the funds in the contract.

• Consider finding and updating an existing PendingStake instead of creating a new one every time.