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

Most of the operations in the LiquidationPool can be bricked by triggering OOG (out of gas) by creating sufficiently large number of pendingStakes

dimulski

Summary

In LiquidationPool.sol we have the following function which allows users to first create a pending stake and later on that pending stake is transformed into a position

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

The EURO token has 18 decimals, and the increasePostion() function allows us to deposit 1 WEI. A malicious user can create hundreds of thousands of pending stake positions, for les than 1 EURO and gas costs. And when in turn

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

is called in order to transform the pending stakes into positions, this function ends up running a for loop over an unbounded array. This array can be made to be sufficiently large to exceed the block gas limit and cause out-of-gas errors and stop the processing of any rewards and assets to the non malicious stakers in the contract.

Impact

This in turns makes the whole contract obsolete as the only function that could be called would be the claimRewards(), but it could only withdraw rewards that were accrued prior to the malicious user stuffing the pendingStakes array with very small positions.

Tools Used

Manual review

Recommendations

Consider setting a minimum value that user needs to deposit in order to call increasePosition() something of the sort of 100e18.

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.