Unauthorized Humanity ID Update via Valid Gateway

[hats-bug-reporter[bot]]

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

Description: Description\ The updateHumanity function in the CrossChainProofOfHumanity.sol contract allows any valid gateway to update the humanity status of any humanity ID. This could potentially lead to unauthorized updates, where a valid gateway could claim or update the humanity status of any humanity ID without proper authorization from the rightful owner.

/**
 * @notice Sends an update of the humanity status to the foreign chain. No need to specify the receiving chain as it'd be known by the gateway
 * @notice Communicates with receiveUpdate function of this contract's instance on the receiving chain
 *
 * @param _bridgeGateway address of the bridge gateway to use
 * @param _humanityId Id of the humanity to update
 */
function updateHumanity(address _bridgeGateway, bytes20 _humanityId) external allowedGateway(_bridgeGateway) {
    (, , , uint40 expirationTime, address owner, ) = proofOfHumanity.getHumanityInfo(_humanityId);
    bool humanityClaimed = proofOfHumanity.isClaimed(_humanityId);

    CrossChainHumanity storage humanity = humanityData[_humanityId];

    require(humanity.isHomeChain || humanityClaimed, "Must update from home chain");

    // isHomeChain is set to true when humanity is claimed
    // It also keeps true value (unless overwritten) if it was set before and humanity is now expired,
    //      thus making it possible to update state of expired / unregistered humanity
    humanity.isHomeChain = true;

    IBridgeGateway(_bridgeGateway).sendMessage(
        abi.encodeWithSelector(
            ICrossChainProofOfHumanity.receiveUpdate.selector,
            owner,
            _humanityId,
            expirationTime,
            humanityClaimed
        )
    );

    emit UpdateInitiated(_humanityId, owner, expirationTime, humanityClaimed, _bridgeGateway);
}

The updateHumanity function is designed to send an update of the humanity status to a foreign chain. However, the function allows any valid gateway (as determined by the allowedGateway modifier) to call it. This means that any authorized gateway can update the humanity status of any humanity ID, regardless of whether they have the rightful authority to do so.

Attack Scenario\ Step 1: Setup

• Assume there are two gateways: GatewayA and GatewayB.
• Both GatewayA and GatewayB are valid gateways as per the allowedGateway modifier.
• User Alice has a humanity ID 0x1234...abcd which is managed by GatewayA.

Step 2: GatewayA Updates Humanity ID

• GatewayA calls the updateHumanity function to update the humanity status of Alice's humanity ID 0x1234...abcd.
• The function checks if GatewayA is an allowed gateway and proceeds with the update.
• The humanity status is successfully updated and sent to the foreign chain.

Step 3: Unauthorized Update by GatewayB

• Now, GatewayB, although valid, should not have the authority to update Alice's humanity ID.
• However, due to the current implementation, GatewayB can call the updateHumanity function with Alice's humanity ID 0x1234...abcd and update its status.
• The function checks if GatewayB is an allowed gateway and proceeds with the update.
• The humanity status of Alice's humanity ID is updated by GatewayB without Alice's consent or authorization.

Step 4: Impact

• This unauthorized update could lead to inconsistencies and potential misuse of the humanity status.
• It undermines the trust and security of the system as any valid gateway can manipulate the humanity status of any user.

this also possible in another function in the crosschainHumnity() function,i will post it in detail in comments

Attachments

1. Proof of Concept (PoC) File

   function updateHumanity(address _bridgeGateway, bytes20 _humanityId) external allowedGateway(_bridgeGateway) {
       (, , , uint40 expirationTime, address owner, ) = proofOfHumanity.getHumanityInfo(_humanityId);
       bool humanityClaimed = proofOfHumanity.isClaimed(_humanityId);
   
       CrossChainHumanity storage humanity = humanityData[_humanityId];
   
       require(humanity.isHomeChain || humanityClaimed, "Must update from home chain");
   
       // isHomeChain is set to true when humanity is claimed
       // It also keeps true value (unless overwritten) if it was set before and humanity is now expired,
       //      thus making it possible to update state of expired / unregistered humanity
       humanity.isHomeChain = true;
   
       IBridgeGateway(_bridgeGateway).sendMessage(
           abi.encodeWithSelector(
               ICrossChainProofOfHumanity.receiveUpdate.selector,
               owner,
               _humanityId,
               expirationTime,
               humanityClaimed
           )
       );
   
       emit UpdateInitiated(_humanityId, owner, expirationTime, humanityClaimed, _bridgeGateway);
   }

   function receiveUpdate(
       address _owner,
       bytes20 _humanityId,
       uint40 _expirationTime,
       bool _isActive
   ) external override allowedGateway(msg.sender) {//@check-anyone able to call this?
       CrossChainHumanity storage humanity = humanityData[_humanityId];
   
       // Clear humanityId for past owner
       delete accountHumanity[humanity.owner];
   
       if (_isActive) {
           accountHumanity[_owner] = _humanityId;
           humanity.owner = _owner;
       } else delete humanity.owner;
   
       humanity.expirationTime = _expirationTime;
   
       // If it received update from another chain `isHomeChain` flag is marked as false
       //         in order to avoid bridging state update of an removed / expired humanity
       humanity.isHomeChain = false;//@check
   
       emit UpdateReceived(_humanityId, _owner, _expirationTime, _isActive);
   }

2. Revised Code File (Optional)

   • To mitigate this issue, additional checks should be implemented to ensure that only the rightful owner or an explicitly authorized entity can update the humanity status.

[clesaege]

This means that any authorized gateway can update the humanity status of any humanity ID, regardless of whether they have the rightful authority to do so.

If they are authorized, they have the rightful authority to do so.

[batmanBinary]

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

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 Attacker Observes the Pending Transaction:

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 ,please let me know if you need any further details.

[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.