hats-finance / Blast-Futures-Exchange-0x97895c329b950755566ddcdad3395caaea395074

0 stars 0 forks source link

An unbounded loop in the pooledDeposit function can potentially cause a self-inflicted denial-of-service (DoS) situation. #69

Open hats-bug-reporter[bot] opened 9 months ago

hats-bug-reporter[bot] commented 9 months ago

Github username: @https://github.com/sekkiat Twitter username: -- Submission hash (on-chain): 0xd3064ee5c12251e0d59ddb10b495019e1a48387128e208c62db9c67fec83856f Severity: low

Description: Description\ The pooledDeposit function in the PoolDeposit.sol does not implement a protection on checking the length of the array, which allows the attacker to create a large array data to DOS the function.

Attack Scenario\ The function pooledDeposit uses an unbounded loop. The loop iterates over the contributions array without any constraint on its length, which could lead to excessive gas consumption and potentially block the contract execution if the array is excessively large.

However, the impact is very low because the array array is separated per user and links to mgs.sender. Therefore, this is consider a self-inflicted DOS attack.\ Attachments

  1. Proof of Concept (PoC) File
    function pooledDeposit(Contribution[] calldata contributions) external {
        uint256 poolId = allocatePoolId();
        uint256 totalAmount = 0;
        //@audit length no protection
        for (uint i = 0; i < contributions.length; i++) {
            Contribution calldata contribution = contributions[i];
            uint256 contribAmount = contribution.amount;
            totalAmount += contribAmount;
            require(contribAmount > 0, "WRONG_AMOUNT");
            require(totalAmount >= contribAmount, "INTEGRITY_OVERFLOW_ERROR");
            uint256 depositId = allocateDepositId();
            emit Deposit(depositId, contribution.contributor, contribAmount, poolId);
        }
        require(totalAmount > 0, "WRONG_AMOUNT");
        emit PooledDeposit(poolId, totalAmount);
        bool success = makeTransferFrom(msg.sender, rabbit, totalAmount);
        require(success, "TRANSFER_FAILED");
    }

Remediation\ • Implementing a limit on the maximum array length or enforcing gas limits within the loop to prevent excessive gas consumption.

alex-sumner commented 9 months ago

As you say, this would be a self inflicted attack. The attacker would achieve nothing but wasting their own gas. However if gas fees are low enough they could cause disruption at a relatively low cost to themselves.

alex-sumner commented 9 months ago

This is the same problem reported in https://github.com/hats-finance/Blast-Futures-Exchange-0x97895c329b950755566ddcdad3395caaea395074/issues/47