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

`pendingStakes` array has no limit and can grow very large, causing out of gas errors

dopeflamingo

Summary

The pendingStakes array in the LiquidationPool contract does not have any limit to its size and there is no upper bound to how large it can get. This can cause "Out of gas" or "Block Gas Limit" errors in loops which are required to iterate over all the indices in this array.

Vulnerability Details

Everytime LiquidationPool::increasePosition is called, this pushes a new stake into the pendingStakes array. As there is no limit to the size, the array can grow infinitely large.

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/LiquidationPool.sol#L134-L142

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);
}

Everytime LiquidationPool::consolidatePendingStakes is called within the contract, it has to iterate over the whole pendingStakes array, if in the worst case this is too large, then any function that calls consolidatePendingStakes will revert due to "Out of Gas" and "Block Gas Limit" errors.

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--;
}
}
}

Impact

This means any function calling consolidatePendingStakes will revert due to "Out of Gas" and "Block Gas Limit" errors. The functions that call this are decreasePosition and distributeAssets, hence meaning users wouldn't be able to decrease their position of the stake and the pool manager won't be able to distribute the assets. Not only this, the gas cost to the array becomes even more expensive.

Tools Used

Manual Review

Recommendations

Implement size checks or limitations on array growth within the increasePosition function.
Alternatively, Consider alternative data structures or pagination techniques to manage large datasets more efficiently and control gas consumption.

Updates

Lead Judging Commences

hrishibhat Lead Judge almost 2 years 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.