malicious user can prevent honest user from calling withdrawrequest

[hats-bug-reporter[bot]]

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

Description:

Report

a honest user can invoke withdrawRequest as long as the following requirements are true:

   function withdrawRequest() external {
        bytes20 humanityId = accountHumanity[msg.sender];
        Humanity storage humanity = humanityData[humanityId];
        uint256 requestId = humanity.requestCount[msg.sender] - 1;
        Request storage request = humanity.requests[requestId];
=>        require(request.status == Status.Vouching);

        delete humanity.requestCount[msg.sender];
        request.status = Status.Resolved;

        // Automatically withdraw for the requester.
        withdrawFeesAndRewards(payable(msg.sender), humanityId, requestId, 0, 0);

        emit RequestWithdrawn(humanityId, requestId);
    }

So as long as the status is still in Vouching a user is able to invoke this function.

Now lets proceed with the following scenario;

• Alice (Honest user)

• Bob (Malicious user)

• Alice proposes a request and fully funds it herself

• Alice wants to revoke her request for example; since she was unable to find sufficient vouchers, so decides to call withdrawRequest, this is still possible since the Status is still Vouching

• Bob notices this and frontruns this call by calling AdvanceState

 function advanceState(
        address _claimer,
        address[] calldata _vouches,
        SignatureVouch[] calldata _signatureVouches

Bob will leave the _vouches & _signatureVouches array empty, note that the comment also reflects this possibility as stating it is optional as per this commentary:

//     *  @param _vouches Array of users whose vouches to count (optional).
//     *  @param _signatureVouches Array of EIP-712 signatures of struct IsHumanVoucher (optional).

AdvanceState will now execute the following lines of code:

    function advanceState(
        address _claimer,
        address[] calldata _vouches,
        SignatureVouch[] calldata _signatureVouches
    ) external {
        bytes20 humanityId = accountHumanity[_claimer];
        Humanity storage humanity = humanityData[humanityId];
        uint256 requestId = humanity.requestCount[_claimer] - 1;
        Request storage request = humanity.requests[requestId];
        require(request.status == Status.Vouching);
        require(
            humanity.owner == address(0x0) || humanity.expirationTime.subCap40(renewalPeriodDuration) < block.timestamp
        );
        require(request.challenges[0].rounds[0].sideFunded == Party.Requester);

        humanity.nbPendingRequests++;
        request.status = Status.Resolving;
        request.challengePeriodStart = uint40(block.timestamp);

        emit StateAdvanced(_claimer);
    }

It will skip the while loop, but will change the Status to Resolving

Ultimately Alice is robbed of her ability to withdraw her request.

this requires 0 to little effort and a bot could be created to repeat this process, the malicious user does not invest anything or risks anything by doing this

Recommendation

it might be more suitable to only allow a creator of a request to call advanceState or alternatively come up with some other logic

[clesaege]

It will skip the while loop, but will change the Status to Resolving

The while loop is never skipped, it only ends when there are enough vouches registered.