The Standard

Summary

The LiquidationPool::consolidatePendingStakes function loops through the pendingStakes array for making checks if they are older than one day. However, the longer the pendingStakes array is, the more checks will be made. This means the gas cost for the staker who stakes earlier will have to pay dramatically less gas than those who stake later.

Even though it consolidates pending stakes into positions and deletes them if they are older than one day, an attacker can create a lot of pendingStakes in a day by calling the increasePosition function continuously by depositing very little TST or EUROs.

Also the decreasePosition ,distributeAssets functions call the consolidateStakes fuction.

An attacker can make the pendingStakes array so big , disrupting the proper functionalities of the protocol.

Vulnerability Details

consolidatePendingStakes function loops through the pendingStakes array:

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L119-L132

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

increasePosition function is used to increase position:

https://github.com/Cyfrin/2023-12-the-standard/blob/main/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);
}

Impact

This causes Denial of Service (DoS).

Tools Used

Manual Review

Recommendations

These are a few recommendations:

1, Allow the stakers to increase their position only after their previous pending stake has been consolidated into their position.

2, Make the increasePosition function accept a minimum amount of TST or EUROs.