Humanity Loss During Cross-Chain Transfers Due to v1 Profile Registration

[hats-bug-reporter[bot]]

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

Description: Description
When a profile transfers its humanity to another chain using the CrossChainProofOfHumanity::transferHumanity function, the humanity on the main chain is deleted to ensure that only one instance of humanity exists across all chains in POHv2.

If a humanity profile is already registered on the receiving chain at the time of transfer, the profile is not created on that chain for the same reason.

During a transfer to the Ethereum chain, the ProofOfHumanityExtended::ccGrantHumanity function is called. This function includes an if condition to check whether the humanity is already claimed on the Ethereum chain:

        // If humanity is claimed, don't overwrite.
        if (
            (humanity.owner != address(0x0) && block.timestamp < humanity.expirationTime) ||
            // If not claimed in this contract, check in fork module too.
@>            forkModule.isRegistered(_account)
        ) return false;

The forkModule contract’s isRegistered function only verifies four conditions:

• Whether the v1 humanity is registered.
• Whether the v1 humanity has expired.
• Whether the v1 humanity was created before the fork time.

• Whether it has been removed from the ForkModule.

  
  @>        if (removed[_submissionID]) return false;
  
      (, uint64 submissionTime, , bool registered, , ) = proofOfHumanityV1.getSubmissionInfo(_submissionID);
  
      uint40 expirationTime = uint40(submissionTime).addCap40(submissionDuration);

@> return registered && block.timestamp < expirationTime && submissionTime < forkTime;

v1 profiles can create humanity on any one of the multiple chains.

A problem arises when a v1 profile that is valid on POHv1 and was created before the fork time creates humanity on other chains before Ethereum and then attempts to transfer its humanity to the Ethereum chain while still valid on v1.

In this scenario, the above-mentioned `if condition` will return false, leading to the profile being unregistered on both the main chain and the Ethereum chain. This results in the loss of humanity for the user in POHv2.

Consequently, all v1 profiles that are valid and created before the fork time, if they first create their humanity on a chain other than Ethereum and then transfer it to the Ethereum chain while their v1 profile is still valid, will have their humanity unregistered on both chains.

**Vulnerability Scenario**  
1. Alice, a user with a valid v1 humanity profile, creates a humanity profile on chain A.
2. Alice decides to transfer her humanity from chain A to the Ethereum chain using the `CrossChainProofOfHumanity::transferHumanity` function.
3. The transfer process involves deleting her humanity on the chain A to ensure that only one humanity profile exists across chains. As part of the transfer process, her humanity profile is deleted from the chain A.
4. The `ccGrantHumanity` function is triggered on the Ethereum chain. This function checks whether humanity is already claimed on Ethereum.
5. Since Alice’s v1 humanity was created before the fork time and is still valid, the `forkModule.isRegistered` check returns true, meaning her profile is considered already claimed on the Ethereum chain.
6. Due to the condition in `ccGrantHumanity`, the function returns `false`, and Alice’s profile is not registered on the Ethereum chain.
7. As her profile was already deleted on chain A during the transfer, Alice is left without an active humanity profile on any chain, leading to the loss of her humanity.

**Mitigation:**  
Add the following lines to the `ProofOfHumanityExtended::ccGrantHumanity` function:
```solidity
function ccGrantHumanity(
    bytes20 _humanityId,
    address _account,
    uint40 _expirationTime
) external onlyCrossChain returns (bool success) {
    Humanity storage humanity = humanityData[_humanityId];

    // If humanity is claimed, don't overwrite.
--    if (
--        (humanity.owner != address(0x0) && block.timestamp < humanity.expirationTime) ||
--        // If not claimed in this contract, check in fork module too.
--        forkModule.isRegistered(_account)
--    ) return false;

++    if (humanity.owner != address(0x0) && block.timestamp < humanity.expirationTime)
++        return false;

++    if (forkModule.isRegistered(_account))
++        forkModule.remove(_account);

    // Must not be in the process of claiming a humanity.
    require(humanityData[accountHumanity[_account]].requestCount[_account] == 0);

[clesaege]

v1 profiles can create humanity on any one of the multiple chains.

They can't do that if they are registered on V1, they would be challenged. As per registration rules:

As per contest rules are excluded:

• Issues about challengers missing some invalid profile submissions/removals (for the purpose of this review, we will assume challengers are perfect and omniscient and that they always challenge invalid actions before the deadline).