The Standard

The Standard
DeFiHardhat
20,000 USDC
View results
Submission Details
Severity: high
Valid

Griefing attack caused by unchecked length of `LiquidationPool::pendingStakes` array and lack of defined minimum stake

Summary

Due to lack of minimum stake required to open a new position using LiquidationPool::increasePosition() the pendingStakes array can be made significantly long with relatively low cost containing positions that equal to 1 wei worth of EUROs & TST. After 24 hour consolidation period other users are unable to open a new position as the increasePosition function calls consolidatePendingStakes where in a for loop all pending stakes are considered and if deadline has passed deletePendingStakes is called with another loop in it.

Vulnerability Details

The issue is caused mainly by lack of minimum position amount as then the cost of any attack of this kind would be much larger than the impact.

  1. An attacker opens 100 new positions which are all added to the pendingStakes array, which costs him ~0.52ETH (considering gas price of 20 gwei).
    Note: This attack is still very cheap, in the PoC it is possible to see the impact of more new positions.

  2. Consolidation period passes ( 24 hours after last attacker's transaction )

  3. A user wanting to create a new position would need to pay ~0.3 ETH to call increasePosition

Proof of Code
PoC is written in Foundry, so to set it up:

  • install hardhat-foundry plugin
    npm i --save-dev @nomicfoundation/hardhat-foundry

  • add the following line to the top of hardhat.config.js file

require("@nomicfoundation/hardhat-foundry");
  • to initialize Foundry run

npx hardhat init-foundry
  • in foundry.toml add the following line to avoid stack too deep errors

viar-ir = true
  • move the utils folder from contracts to the root folder

  • in the test folder create a sub folder and in it create a test file foundryTest.t.sol

  • after pasting the PoC to this file, run the test case with:

forge test --mt test_costOfIncreasingPositionWithAttack -vvv
  • number of stakes generated by the attacker can be easily adjusted by changing ACCOUNTS variable in the specific test case ( not global )

The entire test case is in the Github gist below:
https://gist.github.com/bbl4de/b823f0a9389977fe8f645d473b3f4ad1

Impact

In the comments of a provided test case, there are three different situations presented - 1000 attacker's positions, 500 and 50. This different scenarios are to show that even an attacker with little capital could grief the protocol making the position increase or decrease extremely expensive.

This vulnerability causes not only the cost of increasePosition to be high, but also decreasePosition - run the following test to see the impact:

forge test --mt test_costOfDecreasingPositionWithAttack -vvv

Also, calling LiquidationPool::position function would become expensive for a view function as it calls a private function holderPendingStakes also iterating through the array. For 100 pending stakes generated by the attacker it would cost ~0.009 ETH, while without the attack it costs only ~0.001 ETH, note that it still impacts the other functions and it is on top of it.

forge test --mt test_costOfCallingPositionViewFunctionAfterAttack -vvv

Another test case is provided for a situation where the LiquidationPoolManager decides to call distributeAssets, despite the fact that pendingStakes contains 100 tiny positions that will be consolidated during this call, to restore the functionality for normal users and distribute the assets. It is simplified to show the cost of a call to ONLY distributeAssets function showing the impact of long holders array.

forge test --mt test_costOfAssetDistributionAfterAttack -vvv

This attack vector is so severe, as even more functions in LiquidationPool are impacted when considering a long holders array, but I will skip those as this is considered a known issue.

Tools Used

Manual review, Foundry

Recommendations

Solving this issue might involve significant changes to the protocol due to number of places where pendingStakes and holders arrays are used, which would be beyond the scope of a single finding. However, the first and simplest step of minimazing the likelihod of such attack would be to include a minimum staking amount.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

pendingstake-dos

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

pendingstake-high

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.