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

pendingStakes denial of liquidation sevice

0xBinChook

Summary

LiquidationPool::pendingStakes is an unbounded array that can be sufficiently populated to cause a denial of the liquidation service.

Vulnerability Details

Pending stakes are added to the LiquidationPool in increasePosition()

function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
require(_tstVal > 0 || _eurosVal > 0);
consolidatePendingStakes();
ILiquidationPoolManager(manager).distributeFees();
if (_tstVal > 0) IERC20(TST).safeTransferFrom(msg.sender, address(this), _tstVal);
if (_eurosVal > 0) IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _eurosVal);
@> pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));
addUniqueHolder(msg.sender);
}

Every position increase is pushed into pendingStakes, without restrictions on stake size.

The entire array of pendingStakes is iterated through, with stakes older than one day transitioned to full stakes in consolidatePendingStakes()

function consolidatePendingStakes() private {
uint256 deadline = block.timestamp - 1 days;
for (int256 i = 0; uint256(i) < pendingStakes.length; i++) {
PendingStake memory _stake = pendingStakes[uint256(i)];
if (_stake.createdAt < deadline) {
positions[_stake.holder].holder = _stake.holder;
positions[_stake.holder].TST += _stake.TST;
positions[_stake.holder].EUROs += _stake.EUROs;
deletePendingStake(uint256(i));
// pause iterating on loop because there has been a deletion. "next" item has same index
i--;
}
}
}

No matter the size of pendingStakes, every element is read from storage, with storage writes on their transition to full stakes.

When the pending stake transitions LiquidationPool::deletePendingStake() performs another iteration over pendingStakes

function deletePendingStake(uint256 _i) private {
for (uint256 i = _i; i < pendingStakes.length - 1; i++) {
pendingStakes[i] = pendingStakes[i+1];
}
pendingStakes.pop();
}

LiquidationPool::consolidatePendingStakes() is called in increasePosition(), decreasePosition() and distributeAssets(), while distributeFees() loops over pendingStakes directly.

The size of dynamic pendingStakes being unbounded (no limiting of length by the code), means it can grow to such a size that any calling function will revert due to out of gas or exceeding the block limit size.

If pendingStakes grows to the problem size with one day, LiquidationPool becomes unusable, as the functions to using pendingStakes will revert.

LiquidationPool::distributeFees() is part of the chain of call needed to perform the liquidation of under-collateralised vaults.

Test Case

Gas usage result data has been generated using a Javascript test case

Gas Reporter Setup Transaction gas usage requires installing and configuring the `hardhat-gas-reporter` plugin to TheStandard project.

Add Gas-Reporter for project

npm install -D hardhat-gas-reporter

Add to context

In hardhat.config.ts

Include dependency
require("hardhat-gas-reporter");
Enable reporting
const config: HardhatUserConfig = {
// ....
gasReporter: {
enabled: true,
},
};
​
Test Case Add the test case to liquidationPool.js
describe('denial of service', async () => {
it('consolidate pending stakes', async () => {
const numberOfStakes = 10
const stake = ethers.constants.One
const mintAmount = BigNumber.from(numberOfStakes).mul(stake)
await TST.mint(user2.address, mintAmount)
await TST.connect(user2).approve(LiquidationPool.address, mintAmount)
​
for( let i = 0; i < numberOfStakes; i++){
await LiquidationPool.connect(user2).increasePosition(stake, 0)
}
}).timeout(180000)
})

Run from the project root using:

npx hardhat test --grep "consolidate pending stakes"

Result Data

Altering the numberOfStakes value when running the test and extracting the LiquidationPool result provides the following summary:

·-----------------------------------------|----------------------------|-------------|-----------------------------·
| Solc version: 0.8.17 · Optimizer enabled: false · Runs: 200 · Block limit: 32000000 gas │
··········································|····························|·············|······························
| Methods │
·····················|····················|··············|·············|·············|···············|··············
| Contract · Method · Min · Max · Avg · # calls · usd (avg) │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · - · - · 213806 · 1 · - │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 158279 · 312858 · 238798 · 20 · - │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 158279 · 585688 · 373265 · 50 · - │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 158279 · 1040529 · 600003 · 100 · - │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 158279 · 1950681 · 1054604 · 200 · - │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 158279 · 3772859 · 1964916 · 400 · - │
·····················|····················|··············|·············|·············|···············|··············
| LiquidationPool · increasePosition · 158279 · 5597537 · 2876169 · 600 · - │
·····················|····················|··············|·············|·············|···············|··············

When the # calls to increasePosition increases, the Max gas consumed on each test also increases (in a linear time polynomial).
The increase is due to looping over the unbounded array, with one clear extrapolation: the block limit will be reached with enough calls to increasePosition.

5,597,537 - 3,772,859 = 1,824,678 # Increase in gas for the 200 calls between the 400 and 600 runs
1,824,678 / 200 = 9,123 # Each subsequent call cost 9123 more gas than the previous call
​
32,000,000 - 213,806 = 31,786,194 # Arbiutm gas limit minus the first warm up transaction
31,786,194 / 9,123 = 3,484 # ~3484 increasePosition calls will brick the LiquidationPool on Arbitum

After 3,484 calls to LiquidationPool::increasePosition the array of pending stakes will be large enough to cause any function using LiquidationPool::consolidatePendingStakes() to exceed the Arbitum One block gas limit.

(This is excluding any potential affect of LiquidationPool::deletePendingStake())

Impact

The denial of liquidation services, prevents under-collateralised vaults from being able to liquidated,
in addition to all TST and EUROs held by the LiquidationPool contract trapped within it.

While it is plausible that through organic traffic there could be 3.5K of stakers increasing their position,
lets focus on a malicious actor.

Cheap and easy

Increasing a position can be done for a single gwei of either TST or EUROs plus gas.
The gas cost can be reduced by deploying a batching contract.

A batching contract would execute a loop of increasePosition (a configurable number of times) in a single transaction,
making the attack relatively cheap and easy to execute, while also leaving no time for any preventative measures.

No escape

Being unable to complete a call to consolidatePendingStakes() prevents reducing pendingStakes size, with no other
function available, once the situation has occurred there is no escaping from it.

Tools Used

Manual Review, Hardhat test, Hardhat gas reporter

Recommendations

There are a few options, but your preference will have to include your appetite for redesign.

Considerations

  • Malicious actors

  • Honest stakers

  • o(n) gas scaling for increasing pending stake

  • o(n squared) scaling for deleting pending stake (as part of consolidatePendingStakes())

1) Increase the attack cost (not eliminating the underlying risk)

The quickest option, where a non-trivial amount of value is required for increasing stake.

Partial fix

  • Malicious actors: certain economic circumstances may warrant the addition cost in an attack

Fails to fix

  • Honest stakers

  • o(n) gas scaling for increasing pending stake

  • o(n squared) scaling for deleting pending stake (as part of consolidatePendingStakes())

+ uint256 private constant MINIMUM_EUROS_PENDING_STAKE = 100;
+ uint256 private constant MINIMUM_TST_PENDING_STAKE = 150;
​
function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
- require(_tstVal > 0 || _eurosVal > 0);
+ require(_tstVal > MINIMUM_TST_PENDING_STAKE || _eurosVal > MINIMUM_EUROS_PENDING_STAKE);
consolidatePendingStakes();

2) ReDesign (only prevent malicious actors)

Although a larger redesign could achieve sizable gas efficiencies, a minimal redesign reduces change risk.

Fixes

  • Malicious actors: liquidations can always go through

Partial fix

  • Honest stakers: increasing, decreasing and fee distribution could still be offline (requiring unsticking)

Fails to fix

  • o(n) gas scaling for increasing pending stake

  • o(n squared) scaling for deleting pending stake (as part of consolidatePendingStakes())

2a) Change the liquidation flow

The liquidation process must be decoupled from the promoting of pending stakes to full stakes.

As the Liquidation is essential for maintaining the over-collateralisation of EUROs, no part of the that flow
should be exposed to unbounded lops.

function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
- consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();

2b) Unstuck function

increasePosition(), decreasePosition() and distributeFees() can all still be stuck due to pendingStakes, but assuming these are non-critical
to operations, add a function to parse a sub-set of pendingStakes. After one day anyone could then call the function to reduce the size of pendingStakes by transition stakes that passed their deadline.

function consolidateSubsetOfPendingStakes(uint256 _start, uint256 _end) external{
require(_start < _end, "start-too-big");
require(_end < pendingStakes.length, "end-too-big");
uint256 deadline = block.timestamp - 1 days;
​
for (int256 i = _start; uint256(i) < _end; i++) {
PendingStake memory _stake = pendingStakes[uint256(i)];
if (_stake.createdAt < deadline) {
positions[_stake.holder].holder = _stake.holder;
positions[_stake.holder].TST += _stake.TST;
positions[_stake.holder].EUROs += _stake.EUROs;
deletePendingStake(uint256(i));
// pause iterating on loop because there has been a deletion. "next" item has same index
i--;
}
}

3) Total Overhaul (fix all the problems)

Changes to both the process flow and underlying data structures, providing gas efficient scaling of the protocol.

Fixes

  • Malicious actors

  • Honest stakers

  • o(n) gas scaling for increasing pending stake

  • o(n squared) scaling for deleting pending stake (as part of consolidatePendingStakes())

Although the change set would be too extensive for this issue, the core ideas are:

  • Change the liquidation flow

  • Remove the pendingStakes array

  • Add a FIFO queue of addresses

  • Add a mapping of addresses to an array of PendingStake

  • re-implement the logic of pendingStakes dependent functions

3a) Change the liquidation flow

The liquidation process must be decoupled from the promoting of pending stakes to full stakes.

Similar to the previous option, rather then tagging the processing onto other calls, provide a functions for the sole behaviour,
again with sub-set parsing to account for the potential unbounded set size.

3b) Change the data structure

The contents of pendingStakes will be split into two parts, the FIFO queue and the address to PendingStake mapping

First In First Out (FIFO) queue, e.g. Open Zeppelin Dequeue docs souce, as storage use is optimized, and all operations are O(1) constant time.
The ordering of the queue is by time, with the oldest pending stakes being the earliest entries, which allows for early exits in looping.

Mapping of address to a dynamic array of PendingStake, a store for the pending stakes by user.

Replace the pendingStakes array with the new data structures

+ import "@openzeppelin/contracts/utils/structs/DoubleEndedQueue.sol";
​
- PendingStake[] private pendingStakes;
+ // Time ordered queue of the addresses who are increasing their position
+ DoubleEndedQueue.Bytes32Deque private pendingStakeAddresses
+ mapping(address staker => PendingStake[] pendingStakes) private pendingStakes;

An example of the changes needed to LiquidationPool::increasePosition() are to push the data onto the queue and mappings

function increasePosition(uint256 _tstVal, uint256 _eurosVal) external {
require(_tstVal > 0 || _eurosVal > 0);
- consolidatePendingStakes();
ILiquidationPoolManager(manager).distributeFees();
if (_tstVal > 0) IERC20(TST).safeTransferFrom(msg.sender, address(this), _tstVal);
if (_eurosVal > 0) IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _eurosVal);
- pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));
- addUniqueHolder(msg.sender);
+ DoubleEndedQueue.pushBack(pendingStakeAddresses, msg.sender);
+ pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));
}

Likewise throughout the code, the calling of consolidatePendingStakes() need removing and instead external functions allowing
consolidate by address (so keen users can update themselves) and a function for the consolidate a limited number of users,
as unbounded staker count an now be supported (practically there are storage limits).

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.