Potential frontrunning vulnerability in the `advanceState` function

Github username: -- Twitter username: -- Submission hash (on-chain): 0x60f65a951b741d0157fc9930315e54574513c8e5d2245ef8d955872ce2838b3c Severity: medium

Description: Description Potential frontrunning vulnerability in the advanceState function

Attack Scenario The advanceState function allows anyone to advance the state of a request if certain conditions are met:

function advanceState(
    address _claimer,
    address[] calldata _vouches,
    SignatureVouch[] calldata _signatureVouches
) external {
    // ... (code omitted for brevity)

    while (request.vouches.length < requiredVouches) {
        // ... (code omitted for brevity)
        if (isValid) {
            // ... (code omitted for brevity)
            request.vouches.push(voucherHumanityId);
            voucherHumanity.vouching = true;

            emit VouchRegistered(voucherHumanityId, humanityId, requestId);
        }
        // ... (code omitted for brevity)
    }

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

    emit StateAdvanced(_claimer);
}

A malicious actor could monitor the mempool for advanceState transactions and front-run them by submitting their own transaction with a higher gas price. This could allow them to advance the state of a request before the original caller, potentially manipulating the order of state advancements.

Impact Potential denial of service in processVouches function

Proof of Concept The processVouches function processes a fixed number of vouches in each call:

function processVouches(bytes20 _humanityId, uint256 _requestId, uint256 _iterations) public {
    // ... (code omitted for brevity)
    uint256 endIndex = _iterations.addCap(lastProcessed);
    uint256 vouchCount = request.vouches.length;

    if (endIndex > vouchCount) endIndex = vouchCount;

    while (lastProcessed < endIndex) {
        // ... (code omitted for brevity)
    }
    // ... (code omitted for brevity)
}

If the number of vouches becomes very large, it might become impossible to process all vouches within the block gas limit, potentially locking the contract state.

Recommended Mitigation Steps Implement a dynamic approach to process as many vouches as possible within the gas limit:

function processVouches(bytes20 _humanityId, uint256 _requestId, uint256 _maxIterations) public {
    Request storage request = humanityData[_humanityId].requests[_requestId];
    require(request.status == Status.Resolved);

    uint256 lastProcessed = request.lastProcessedVouch;
    uint256 vouchCount = request.vouches.length;
    uint256 gasLeftStart = gasleft();

    for (uint256 i = 0; i < _maxIterations && lastProcessed < vouchCount; i++) {
        if (gasLeftStart - gasleft() > 15000) {  // Adjust this value based on the average gas cost of processing one vouch
            break;
        }

        // Process vouch logic here
        // ... (code omitted for brevity)

        lastProcessed++;
    }

    request.lastProcessedVouch = uint32(lastProcessed);

    emit VouchesProcessed(_humanityId, _requestId, lastProcessed);
}

This approach ensures that the function processes as many vouches as possible within the gas limit, preventing potential DOS situations.

hats-finance / Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0

Potential frontrunning vulnerability in the `advanceState` function #35