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

LiquidationPool: array size of `pendingStakes` would be increase to huge value by `increasePosition` which cause DOS on multiple places.

ak1

Summary

Array size of pendingStakes increases to huge value such that the traversal of it could cause the DOS issue.

Vulnerability Details

Anyone can call the function increasePosition to increase the TST and EURO. The amount of stake is added in an array.
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)); ------------->> refer here.
addUniqueHolder(msg.sender);
}

Later this pendingStakes would be used in the places as given in the impacted code link section. Lets see on of the place where the traversal of pendingStakes is done.

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

​
function distributeFees(uint256 _amount) external onlyManager {
uint256 tstTotal = getTstTotal();
if (tstTotal > 0) {
IERC20(EUROs).safeTransferFrom(msg.sender, address(this), _amount);
for (uint256 i = 0; i < holders.length; i++) {
address _holder = holders[i];
positions[_holder].EUROs += _amount * positions[_holder].TST / tstTotal;
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
pendingStakes[i].EUROs += _amount * pendingStakes[i].TST / tstTotal;
}
}
}

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

function holderPendingStakes(address _holder) private view returns (uint256 _pendingTST, uint256 _pendingEUROs) {
for (uint256 i = 0; i < pendingStakes.length; i++) {
PendingStake memory _pendingStake = pendingStakes[i];
if (_pendingStake.holder == _holder) {
_pendingTST += _pendingStake.TST;
_pendingEUROs += _pendingStake.EUROs;
}
}
}

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

function getTstTotal() private view returns (uint256 _tst) {
for (uint256 i = 0; i < holders.length; i++) {
_tst += positions[holders[i]].TST;
}
for (uint256 i = 0; i < pendingStakes.length; i++) {
_tst += pendingStakes[i].TST;
}
}

Impact

Increasing the array size indefinitely would lead to permanent DOS value.
This would impact the functioning of LiquidationPool contract.

Tools Used

Manual review.

Recommendations

We would suggest to add a cap on the number of pendingStakes for an user.
Once the cap is reached, user can wait till clearing of this array to add more in the pendingStake.

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.