Frontrunning Exploit in claimHumanity Function Affecting accountHumanity Mapping

[hats-bug-reporter[bot]]

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

Description: Description\ The claimHumanity function in the ProofOfHumanity.sol contract does not verify that the provided _humanityId corresponds to the caller's address. This oversight can potentially allow a malicious user to observe the mempool and call the claimHumanity function with a specific _humanityId before the legitimate user does. This affects the accountHumanity mapping, associating the malicious actor's address with the _humanityId.

function claimHumanity(bytes20 _humanityId, string calldata _evidence, string calldata _name) external payable {
    Humanity storage humanity = humanityData[_humanityId];

    require(_humanityId != 0);
    require(!isHuman(msg.sender));
    require(humanity.owner == address(0x0) || humanity.expirationTime < block.timestamp);

    uint256 requestId = _requestHumanity(_humanityId);

    emit ClaimRequest(msg.sender, _humanityId, requestId, _name);
    emit Evidence(
        arbitratorDataHistory[arbitratorDataHistory.length - 1].arbitrator,
        uint256(keccak256(abi.encodePacked(_humanityId, requestId))),
        msg.sender,
        _evidence
    );
}

_requestHumanity Function:

function _requestHumanity(bytes20 _humanityId) internal returns (uint256 requestId) {
    // Human must not be in the process of claiming a humanity.
    require(humanityData[accountHumanity[msg.sender]].requestCount[msg.sender] == 0);

    Humanity storage humanity = humanityData[_humanityId];

    requestId = humanity.requests.length;

    Request storage request = humanity.requests.push();
    request.requester = payable(msg.sender);

    uint256 arbitratorDataId = arbitratorDataHistory.length - 1;
    request.arbitratorDataId = uint16(arbitratorDataId);

    // Use requestCount like this in order to avoid having referencing the claimer's pending request as 0 at any point.
    humanity.requestCount[msg.sender] = requestId + 1;
    accountHumanity[msg.sender] = _humanityId;

    ArbitratorData memory arbitratorData = arbitratorDataHistory[arbitratorDataId];
    uint256 totalCost = arbitratorData.arbitrator.arbitrationCost(arbitratorData.arbitratorExtraData).addCap(
        requestBaseDeposit
    );
    _contribute(_humanityId, requestId, 0, 0, Party.Requester, totalCost);
}

Attack Scenario\

Legitimate User (Alice):

Alice wants to claim humanity with the following details:

• _humanityId: 0x1234567890abcdef1234567890abcdef12345678
• _evidence: "Alice's proof of humanity"
• _name: "Alice" Alice prepares to call the claimHumanity function with the above details.

Malicious User (Bob):

• Bob observes the mempool and sees Alice's pending transaction to call claimHumanity.
• Bob quickly calls the claimHumanity function with the same _humanityId before Alice's transaction is mined.

Outcome:

• Bob's transaction is mined first, and the accountHumanity mapping is updated to associate Bob's address with _humanityId.
• Alice's transaction fails because the _humanityId is now associated with Bob.

Attachments

1. Proof of Concept (PoC) File

This issue allows a malicious user to affect the accountHumanity mapping, potentially leading to:

Unauthorized association of _humanityId with a malicious actor's address. Denial of service for legitimate users attempting to claim humanity.

2. Revised Code File (Optional)

[clesaege]

Indeed, it is possible to claim a humanity not corresponding to your address if you already had this humanityId previously (note that you could have it on another chain, this is why we shouldn't check this humanityId has ever been used as it could have been used on another platform).

Therefore, this isn't checked at code level, but checked by challengers. As per the PoH Registry Policy:

The default humanity being the bytes20 of your address.

So if you try to register with the humanityId of someone else, your request should be denied.

[clesaege]

After double checking it appears that the registration rules themselves do not define what is the default Humanity ID(you would have to read the comments of the smart contract to make sense of the "default Humanity ID") so we'll change that and attribute a Low reward (it doesn't really fit the scope but since we end up doing a change we should reward, the reward will be added to the total pool to ensure it cannot lead to someone finding vulns in scope getting less).