Summary

Denial of service due to increasing gas price when a bad-actor(s)-stake-holder very frequently increases and decreases their position on the same day

Vulnerability Details

This revolves around consolidatePendingStakes() function which is present in the file contracts/LiquidationPool.sol. You will find that it is only called from 3 functions - increasePosition, decreasePosition and distributeAsset all of which are present in the same file.

Problem is that we loop through the whole array of pendingStakes and this can increase the cost linearly on a given day. Of course also, on the next day it will clear up all the previous days' pending stakes but even then the first caller on a new day is at a disadvantage.

The bigger problem though is a bunch of bad actors stake holders could choose to rapidly decrease/increase their position a bunch of times at the start of every day (thereby accumulating pending stakes) to the point where it becomes way too expensive in terms of gas and time for the legitimate stake holders to change their position. This way of locking people can get really annoying.

Proof of code:

Step 1:

Install the gas tracker library

npm install hardhat-gas-trackooor --save-dev

Step 2:

Make the following changes to hardhat.config.js

In your tests/liquidationPool.js, make the foloowing adjustments which will mock the bad actors stake holders

Import statements

Now, inside the beforeEach of describe('LiquidationPool') attach Gas Tracker

Inside the describe('claim rewards') make the following changes

Now if you run this you will find that the gas cost increase lineraly like this. (look inside claimRewards test suite)

Also look what happens to the gas cost for the first tx on the next day.

Impact

All the stakeholders can face denial of service even with just one bad actor. Once the positions are locked, the price of stablecoins can potentially be tampered with.

Tools Used

Intense staring at the code base

Recommendations

In such a situation I would recommend the use of Chainlink's AutomationCompatibleInterface which can checkUpkeep and performUpkeep. The advantage you would have is that inside the checkUpkeep you could specify the condition to be something like # of post deadline pendingStakes > 20 so that we clean house every X amount of pendingStakes rather than X amount of days. (Oh, and the number of post deadline pending stakes can be found by binary search on the pending stakes array for the deadline amount - this would work since it's a sorted array)