The Standard

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

Unbounded loop length can result in DOS

Summary

There are some arrays that can easily grow to a large amount. When these arrays are used in unbounded loops, they may lead to a temporary denial-of-service (DoS) of these functions.

Vulnerability Details

While the sponsor has acknowledge that holders has no length check, and it could affect many other functions through the contract. There is also another variable pendingStake which could easily be used to DOS many of the core functions within LiquidationPool.

This can be seen within the increasePosition() function that pushes a new pendingStake object to the array.

pendingStakes.push(PendingStake(msg.sender, block.timestamp, _tstVal, _eurosVal));

While this array does shrink and can be reduced in size, it is also very easy to increase it's size with the increasePosition() function.

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

An attacker can easily call this function N number of times while transferring/increasing their position by very small amounts such as 1 wei of either the TST token or the EUROs token.

Proof of concept

  1. Attacker calls increasePosition() N times, while sending very small amounts of TST token or EUROs.

  2. This is repeated every second until functions that rely on looping this array such as increasePosition() function reverts (out of gas).

  3. Then when functions such as increasePosition(), decreasePosition(), or distributeAssets() is called, the transaction will fail, as it calls the consolidatePendingStakes().

  4. As a result, the array will actually never shrink as this is the only function that does it, and essentially will be permanently DOS.

Impact

The contract will essentially be DOS, with any remaining rewards/staked tokens. Once an attacker has accumulated enough positions within the array, crucial function calls within the contract will no longer be able to be called permanently.

Tools Used

Manual Review

Recommendations

To solve this issue, the protocol could, reduce the duration to consolidate the pending stakes, so that it will be easier to delete items from the pendingStake array. The protocol could also limit the number of new pendingStake position a user can have within a time period.

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.