Malicious user can DoS claim humanity at vouching state

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x9b5ca14e13e3ca7cb1c1604dfcf6da3a45396b7a53187095ea58e0dc035cb562 Severity: high

Description: Description\ Malicious user can DoS claim humanity at vouching state

Attack Scenario\ fundRequest enables users to fund a request during vouching state, however if a request is fully funded a user can call this function with zero msg.value then _contribute would set round.sideFunded = Party.None whcih DoS the advance state function because of the following check:

require(request.challenges[0].rounds[0].sideFunded == Party.Requester);

Scenario : 1- a request is fully funded its 0 round round.sideFunded = Party.Requester 2- Malicious user calls the fundRequest function with zero msg.vale

 function fundRequest(bytes20 _humanityId, uint256 _requestId) external payable {
        Request storage request = humanityData[_humanityId].requests[_requestId];
        require(request.status == Status.Vouching);

        ArbitratorData memory arbitratorData = arbitratorDataHistory[request.arbitratorDataId];
        uint256 totalCost = arbitratorData.arbitrator.arbitrationCost(arbitratorData.arbitratorExtraData).addCap(
            requestBaseDeposit
        );

        _contribute(_humanityId, _requestId, 0, 0, Party.Requester, totalCost);
    }

then the _contribute would be performed, contribution has 0 value and required amount also would be zero, and since round.sideFunded == Party.Requester so it updates round.sideFunded to Party.None which prevents user to advanceState and claim its humanity

        uint256 contribution = msg.value;
        uint256 requiredAmount = _totalRequired.subCap(
            _side == Party.Requester ? round.paidFees.forRequester : round.paidFees.forChallenger
        );
        if (requiredAmount <= msg.value) {
            contribution = requiredAmount;
            remainingETH = msg.value - requiredAmount;

            paidInFull = true;
            round.sideFunded = round.sideFunded == Party.None ? _side : Party.None;
        }

[clesaege]

I'll need to double check, but it does look valid.

It is however doesn't fit the definition of high:

Issues that allows a takeover of the registry:

• Issues allowing the creation within a reasonable period of time (<1 month) of a very large (> than the amount of profiles already registered) amount of registered identities (ex: Being able to bypass the whole vouching and challenger system to create as many profiles as you wish).
• Issues allowing to take the Humanity Id of any user in a platform the user is registered (ex: Being able to change your Humanity Id to that of any user).
• Issues allowing to illegally remove a very large (>50%) number of registered profiles (ex: Being able to illegally remove any user from the registry).

None of those being fulfilled.

I think it does however fulfilled the Medium definition

Issues that can lead to improper profile creation/removal but only at small scale or to steal funds.

• Issues allowing to remove a profile in an illegitimate manner in a particular situation (ex: prevent the renewal of a profile).

It doesn't steal nor destroy funds (the requests can still be withdrawn and the full deposit refunded), but if exploited by a motivated actor, this actor could set up a MEV bot to systematically "cancel" the submissions such that they can be withdrawn. It would be possible to respond to that by setting up a smart contract to fund and advance state in the same TX, but that would still be a disruption of the system.

[aktech297]

how this uint256 totalCost = arbitratorData.arbitrator.arbitrationCost(arbitratorData.arbitratorExtraData).addCap( requestBaseDeposit ); is possible be zero ?

[MehdiKarimi81]

how this uint256 totalCost = arbitratorData.arbitrator.arbitrationCost(arbitratorData.arbitratorExtraData).addCap( requestBaseDeposit ); is possible be zero ?

totalCost is not zero but calculated requiredAmount is zero, since the round was funded so required amount would be zero

uint256 requiredAmount = _totalRequired.subCap(
            _side == Party.Requester ? round.paidFees.forRequester : round.paidFees.forChallenger
        );

[clesaege]

It's not totalCost being 0, but requiredAmount.

[clesaege]

Adding Danil answer:

You can fund 0 yourself after this and it'll update the status to Requester from which you can proceed to the next state unless the profile is specifically targeted

So it's actually easier to counter, we just need to fund 0 again.

[MehdiKarimi81]

Adding Danil answer:

You can fund 0 yourself after this and it'll update the status to Requester from which you can proceed to the next state unless the profile is specifically targeted

So it's actually easier to counter, we just need to fund 0 again.

Yes and as you mentioned malicious actor could set up a MEV bot to systematically target a profile, unless the profile respond by setting up a smart contract to fund and advance state in the same TX but we can't expect normal users to set up a smart contract so it would disrupt the system.

[clesaege]

Should be fixed by https://github.com/Proof-Of-Humanity/proof-of-humanity-v2-contracts/pull/61