Malicious Actor Can Hijack Another User's `humanityId`

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

Description: Description:
A malicious actor can exploit the claimHumanity function to claim the same humanityId that another user is attempting to claim. Once both users pass the challenge period, they can use the executeRequest function to claim humanity, even if they have the same humanityId. This results in the overwriting of the first user’s humanity claim, causing them to lose their humanityId unfairly.

Reasons for the Exploit:

ProofOfHumanity Contract's Mapping: In the ProofOfHumanity contract, the accountHumanity variable is used to map an address to a humanityId. When a user claims humanity using the claimHumanity function with a specific humanityId, the _requestHumanity function updates the accountHumanity mapping with the user's address:
```
accountHumanity[msg.sender] = _humanityId;
```
However, another user can invoke the claimHumanity function with the same humanityId. The accountHumanity mapping will then store this second user’s address with the same humanityId. Since there are no restrictions preventing this, multiple users can have the same humanityId in the accountHumanity mapping.
Simultaneous Progression Through States: With multiple users claiming the same humanityId, both users can simultaneously enter the vouching state, each planning to claim humanity with the same humanityId. As both users submit valid proof and evidence, they can each secure the necessary votes to advance to the next state by calling the advanceState function. This function checks only the status of each request and whether the provided humanityId has an owner. If neither user has any existing humanity and the humanityId is unclaimed, both can easily progress through this state.
Challenge System Bypass: Given that both users' profiles are valid, there is no reason for a challenger to dispute their claims. This oversight allows both users to pass through the challenge system unchallenged.
Race Condition in executeRequest Function: When a user calls the executeRequest function to finalize their humanity claim, an issue arises if another user has also passed the challenge system. The second user can also call the executeRequest function with the same humanityId to claim it. The executeRequest function only checks the request's status and the challenge period; it does not check if another user has already claimed humanity with the same humanityId. As a result, the second user can overwrite the first user's claim, unfairly causing the first user to lose their humanityId.

Key Issue Highlight: During the entire process, both users move through the claiming states (vouching, resolving, resolved) nearly in parallel. The overlapping or closely ending challenge periods create a scenario where challengers may miss the fact that two users are claiming the same humanityId. This creates a blind spot where, by the time the first user completes their claim, the second user is already outside the challenge period and cannot be contested.

As noted in the out-of-scope issue: A user losing their profile if someone else manages to register a profile with the same ID on the target platform (this is not prevented by code, but by challenging duplicate profiles, even if they are on different platforms).

While challengers can dispute duplicate profiles, they require an existing registered profile to identify the new request as a duplicate. Without an already registered profile, there is no basis for challengers to issue a challenge.

Therefore, since neither user is registered while in the Resolving state, challengers cannot contest them, even if both attempt to claim the same humanityId.

This loophole enables users to bypass the challenge system and claim the same humanityId.

Exploitation:

A malicious user can exploit this vulnerability to take over another user's humanity. The attacker would only need to call the createHumanity function using the humanityId of a new user who is just beginning the claiming process. By providing valid evidence and creating a legitimate profile, the attacker could avoid challenges and eventually hijack the other user's humanity.

To maximize their chances, the attacker can challenge the new user's claim using any 4 reasons such as "incorrect evidence" or "invalid humanity," delaying user's process and preventing him from executing his request before malicious user's own challenge period ends. Once the delay is sufficient, malicious user can proceed with his claim unopposed, successfully hijacking the new user's humanityId.

Attack Scenario:

Bob, an innocent user, initiates a claim for a humanityId using the claimHumanity function. The accountHumanity mapping is updated with Bob's address associated with the humanityId.
Alice, a malicious actor, notices Bob's activity and decides to exploit the process. Alice submits a claim for the same humanityId using the claimHumanity function. The accountHumanity mapping is updated to include Alice's address with the same humanityId.
Both Bob and Alice proceed to the vouching state, gathering vouches to claim the same humanityId. Since both users provide valid proof and evidence of their humanity, they obtain the necessary votes and advance to the next state by calling the advanceState function.
Given that Bob and Alice's profiles appear valid and there are no grounds for a challenge (as challengers cannot detect duplicate profiles without a registered entry), both users pass the challenge period without any objections.
Bob, having passed the challenge period, calls the executeRequest function to finalize his humanity claim.
Shortly after, Alice, whose request has also passed through the same unchallenged process, calls the executeRequest function with the same humanityId.
As a result, Alice's claim overwrites Bob's, causing Bob to lose his humanityId unfairly.

Mitigation:

Ensure that the executeRequest function not only verifies the request status and challenge period but also checks for existing claims on the same humanityId to prevent overwriting.
Consider adding a mechanism to handle situations where multiple users attempt to claim the same humanityId simultaneously.

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

Malicious Actor Can Hijack Another User's `humanityId` #90