any owner can receive anyone's humanity ID

[hats-bug-reporter[bot]]

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

Description: Description\ The receiveTransfer function in CrossChainProofOfHumanity.sol has a potential vulnerability that allows any owner to receive anyone's humanity ID. This vulnerability arises because the function does not adequately verify that the _owner parameter is the intended recipient of the humanity transfer. An attacker or malicious owner could exploit this by front-running a legitimate transfer and calling the receiveTransfer function with their own address as the _owner, thereby claiming the humanity ID for themselves.

Attack Scenario\

Legitimate Transfer Initiation:

• A legitimate user, Alice, initiates a transfer of a humanity ID 0x123 to her address 0xAlice.
• The transfer is processed on the foreign proxy, and a corresponding call to the receiveTransfer function on the main chain is expected.

Monitoring the Blockchain:

• An attacker, Bob, monitors the blockchain for pending humanity transfers.
• Bob sees the pending transfer of humanity ID 0x123 to 0xAlice.

Front-Running the Transfer:

• Before the legitimate transfer is finalized, Bob quickly calls the receiveTransfer function with the following parameters:
• _owner: 0xBob (attacker's address)
• _humanityId: 0x123 (the humanity ID being transferred)
• _expirationTime: The same expiration time as the legitimate transfer
• _transferHash: The same transfer hash as the legitimate transfer

Execution of receiveTransfer Function:

• The receiveTransfer function is executed with Bob's parameters:

function receiveTransfer(
    address _owner,
    bytes20 _humanityId,
    uint40 _expirationTime,
    bytes32 _transferHash
) external override allowedGateway(msg.sender) {
    require(!receivedTransferHashes[_transferHash]);

    bool success = proofOfHumanity.ccGrantHumanity(_humanityId, _owner, _expirationTime);

    CrossChainHumanity storage humanity = humanityData[_humanityId];
    delete accountHumanity[humanity.owner];

    if (success) {
        accountHumanity[_owner] = _humanityId;
        humanity.owner = _owner;
        humanity.expirationTime = _expirationTime;
        humanity.isHomeChain = true;
        humanity.lastTransferTime = uint40(block.timestamp);
    }

    receivedTransferHashes[_transferHash] = true;
    emit TransferReceived(_humanityId, _owner, _expirationTime, _transferHash);
}

Granting Humanity to the Attacker:

• The receiveTransfer function calls ccGrantHumanity with Bob's address:

function ccGrantHumanity(
    bytes20 _humanityId,
    address _account,
    uint40 _expirationTime
) external onlyCrossChain returns (bool success) {
    Humanity storage humanity = humanityData[_humanityId];

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

    require(humanityData[accountHumanity[_account]].requestCount[_account] == 0);

    humanity.owner = _account;
    humanity.expirationTime = _expirationTime;
    accountHumanity[_account] = _humanityId;

    emit HumanityGrantedDirectly(_humanityId, _account, _expirationTime);

    return true;
}

Humanity Not Claimed:

• The ccGrantHumanity function checks if the humanity ID 0x123 is already claimed. Since it is not, the function proceeds to grant the humanity to Bob.

Updating State:

The humanity ID 0x123 is now associated with Bob's address 0xBob. The state is updated to reflect Bob as the owner of the humanity ID.

Attachments

1. Proof of Concept (PoC) File

function receiveTransfer(//@audit-any owner can receive anyone's humanity
        address _owner,
        bytes20 _humanityId,
        uint40 _expirationTime,
        bytes32 _transferHash
    ) external override allowedGateway(msg.sender) {
        // Once transfer hash is flagged as received it is not possible to receive the transfer again
        require(!receivedTransferHashes[_transferHash]);

        // If humanity is claimed on the main contract it will return false and not override the state
        // Otherwise requires _owner to not be in process of claiming a humanity
        bool success = proofOfHumanity.ccGrantHumanity(_humanityId, _owner, _expirationTime);

        CrossChainHumanity storage humanity = humanityData[_humanityId];

        // Clear human humanityId for past owner
        delete accountHumanity[humanity.owner];

        // Overriding this data in case it is outdated
        if (success) {
            accountHumanity[_owner] = _humanityId;

            humanity.owner = _owner;
            humanity.expirationTime = _expirationTime;
            humanity.isHomeChain = true;
            humanity.lastTransferTime = uint40(block.timestamp);
        }

        receivedTransferHashes[_transferHash] = true;

        emit TransferReceived(_humanityId, _owner, _expirationTime, _transferHash);
    }

2. Revised Code File (Optional)

• To mitigate this issue, add an additional check to ensure that the _owner parameter in receiveTransfer matches the intended recipient.

[clesaege]

An attacker or malicious owner could exploit this by front-running a legitimate transfer and calling the receiveTransfer function with their own address as the _owner, thereby claiming the humanity ID for themselves.

We check that the sender is an allowed gateway so an attacker call would revert.

[AresAudits]

@clesaege ,I think there is misunderstanding.please consider the below example

below is how the bridge works from the docs

• User Action: User calls a function on the originating contract.
• Message Preparation: Originating contract calls requireToPassMessage() on the Foreign Bridge contract.
• Event Emission: Foreign Bridge emits UserRequestForAffirmation.
• Relay and Validation: Validators relay the message and collect signatures.
• Execution: Validator calls executeAffirmation() on the Home Bridge contract.
• Function Call: Home Bridge calls the function on the target contract on the destination chain.

now let's understand the attack scenario

Legitimate User Action:

• Alice is the legitimate owner of a humanity ID (humanityId).
• Alice wants to receive her humanity ID to another chain.
• Alice initiates the transfer by calling a function on the originating contract, which in turn calls requireToPassMessage() on the Foreign Bridge contract with the following parameters:
• _owner: Alice's address (0xAlice)
• _humanityId: 0x12345
• _expirationTime: 1672531199 (some future timestamp)
• _transferHash: 0xabcdef (hash of the transfer details)

Exploitation by Attacker

• Bob, an attacker, observes Alice's pending transaction on the blockchain.
• Bob sees the parameters Alice used: _owner, _humanityId, _expirationTime, and _transferHash. Attacker Front-Runs the Transaction:

Bob quickly prepares his own transaction to the Foreign Bridge contract, calling requireToPassMessage() with the following parameters:

• _owner: Bob's address (0xBob)
• _humanityId: 0x12345 (same as Alice's)
• _expirationTime: 1672531199 (same as Alice's)
• _transferHash: 0xabcdef (same as Alice's)
• Bob submits his transaction with a higher gas fee to ensure it gets mined before Alice's transaction.

Executing the Transfer on the Home Side:

• The Home Bridge contract decodes the message and calls receiveTransfer() on the CrossChainProofOfHumanity contract with Bob's parameters.
• The receiveTransfer() function sets Bob as the owner of the humanity ID (0x12345), even though Bob is not the legitimate owner.

Result

• Bob successfully front-ran Alice's transaction and received ownership of Alice's humanity ID on the other chain.
• Alice's legitimate transfer is now invalid because the receiveTransfer function has already processed the transfer hash (0xabcdef).

[clesaege]

Bob quickly prepares his own transaction to the Foreign Bridge contract, calling requireToPassMessage() with the following parameters:

This will not satisfy require(amb.messageSender() == foreignGateway, "!foreignGateway"); so won't work.