hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

Denial of service for `LM_PC_Bounties_v1.sol::verifyClaim` function due to unbounded contributors length. #41

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0x9d5ecfac9dce86adb360222a580336918a27ed62e53ad5e7a5a445c615ad770f Severity: medium

Description: Description

The contributors length in LM_PC_Bounties_v1.sol is unbounded, and the length of contributors is looped in LM_PC_Bounties_v1.sol::verifyClaim. If the length of contributors is too large, iterating over them will become very costly and can result in a gas cost that is over the block gas limit. This will mean that a transaction cannot be executed anymore, causing functions such as LM_PC_Bounties_v1.sol::verifyClaim in a state of DoS.

Attack Scenario

Denial of service in functions such as LM_PC_Bounties_v1.sol::verifyClaim if the contributors length is large. As a result, the legitimate payments cannot be processed properly and causing loss of funds to user.

Attachments

NA

  1. Proof of Concept (PoC) File

Foundry Results:

The gas cost when total_contributors set to 10:

Running 1 test for test/modules/logicModule/paymentClient/LM_PC_Bounties_v1.t.sol:LM_PC_BountiesV1Test
[PASS] test_DoSInProcessPayments() (gas: 2784763)
Logs:
  Gas Used to call verifyClaim function:  1316156

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.96ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

The gas cost when total_contributors set to 1000:

Running 1 test for test/modules/logicModule/paymentClient/LM_PC_Bounties_v1.t.sol:LM_PC_BountiesV1Test
[PASS] test_DoSInProcessPayments() (gas: 234524719)
Logs:
  Gas Used to call verifyClaim function:  120558832

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 114.31ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Noticed that the gas cost increase exponentially when more orders are created.

  1. Revised Code File (Optional)

Consider checking reasonable minimum value for contributors parameter in Contributors[] struct to prevent putting protocol in the state of DoS.

FHieser commented 3 weeks ago

Unbounded Contributors yes, but DoS is not happening, because multiple claims can be created for a single bounty

0xmahdirostami commented 2 weeks ago

thank you @FHieser , yes the issue is invalid, besides that, it creates a lot of gas consumption for addClaim function as well. if the list is too large, the addClaim function reverts as well, besides that bountyManager is trusted.

    function test_DoSInProcessPayments() public {
        uint total_contributors = 1000;
        ILM_PC_Bounties_v1.Contributor[] memory contributors = new ILM_PC_Bounties_v1.Contributor[](total_contributors);
        for (uint i = 0; i < total_contributors; i++){
            contributors[i].addr = address(uint160(uint256(keccak256(abi.encodePacked(i)))));
            contributors[i].claimAmount = 1;

        }

        bountyManager.addBounty(1, 100_000_000, bytes("")); // Id 1

        uint gasBefore1 = gasleft();
        bountyManager.addClaim(1, contributors, bytes("")); // Id 2
        uint gasAfter1 = gasleft();
        console.log("Gas Used to call addClaimfunction: ", gasBefore1 - gasAfter1);

        // notClaimed
        _token.mint(address(_fundingManager), 100_000_000);
        uint gasBefore = gasleft();
        bountyManager.verifyClaim(2, contributors);
        uint gasAfter = gasleft();
        console.log("Gas Used to call verifyClaim function: ", gasBefore - gasAfter);
    }
Logs:
[PASS] test_DoSInProcessPayments() (gas: 234525792)
Logs:
  Gas Used to call addClaim function:  113234911
  Gas Used to call verifyClaim function:  120559045