The Standard

Summary

A permanent DoS of the LiquidationPool can happen if a malicious user sends too many pending requests.

Vulnerability Details

In LiquidationPool contract we use the increasePosition function to stake our EURO or TST. After we submit the request our data is stored in the pendingStakes array and when 1 day passes our position mapping is updated and the pending stake is deleted. An attacker can send multiple staking requests with very little staking value provided and by doing so the pendingStakes array will increase by size a lot and when a good user tries to stake his own tokens (after 1 day passes of the attacker's transactions), the transaction will fail with out of gas issue. This leaves the protocol bricked as both decreasePosition and increasePosition functions call the consolidatePendingStakes function that would revert with out of gas error. As a result all the funds will be stuck in the contract.

Exploit scenario

1. The initial state of the contract: 200TST tokens and 10,000EUROs tokens

2. An attacker calls increasePosition function providing as arguments 0TST and 1EUROs (this equals 1 wei of EUROs since it has 18 decimals)

3. His data is stored in the pendingStakes array

4. The attacker repeatedly calls the same function with the same parameters until the pendingStakes array becomes so big that looping through it would cause an out of gas error. (this transactions are done in a very short period amount of time)

5. 1 day passes (in order for the pending stakes to consolidate)

6. A good user wants to withdraw his staked amounts and he calls decreasePosition which calls consolidatePendingStakes and loops through all the pending stakes that were pushed in the array (remember that it also writes to storage which is a very expensive operation) and starts to consolidate each stake, however since this array is so big in size the transaction reverts as there is no gas left, leaving the funds of all the users stuck.

It doesn't take that much for an attacker to pull this off since there are a lot of for loops. For example to decrease a position you would have to loop through the pendingStakes array 4 times.

Impact

Permanent DoS of the protocol leaving funds stuck

Tools Used

Manual Review

Recommendations

Avoid using for loops at all cost, I would recommend changing the pendingStakes array to a mapping where the address of the user is the key and PendingStake struct is the value. By doing this you can eliminate the for loop in consolidatePendingStakes function.