Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No Verification for Humanity ID Correspondence to Owner in receiveUpdate Function #147

Open
hats-bug-reporter bot opened this issue Sep 2, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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

Description:
Description
The receiveUpdate function in the CrossChainProofOfHumanity contract does not verify that the provided _humanityId corresponds to the _owner before updating the humanity data. This could potentially be exploited if a malicious actor were able to call this function through an approved gateway with incorrect data, leading to incorrect or Incorrect updates to the humanity data.

Attack Scenario
Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File

The affected function is receiveUpdate in the CrossChainProofOfHumanity contract:

 function receiveUpdate(
        address _owner,
        bytes20 _humanityId,
        uint40 _expirationTime,
        bool _isActive
    ) external override allowedGateway(msg.sender) {//@audit-no verification of owner
        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-this can be used to trick transferHuanity() check?

        emit UpdateReceived(_humanityId, _owner, _expirationTime, _isActive);
    }
  • Expected Behavior :
    The function should verify that the _humanityId corresponds to the _owner before updating the humanity data. If the verification fails, the function should revert with an appropriate error message.

  • Actual Behavior :
    The function updates the humanity data without verifying that the _humanityId corresponds to the _owner, potentially leading to incorrect or malicious updates.

Note

  1. Revised Code File (Optional)
  • Add a verification step in the receiveUpdate function to ensure that the _humanityId corresponds to the _owner before updating the humanity data.
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Sep 2, 2024
@clesaege
Copy link

clesaege commented Sep 3, 2024

This could potentially be exploited if a malicious actor were able to call this function through an approved gateway with incorrect data, leading to incorrect or Incorrect updates to the humanity data.

So it looks like this report is saying is "If there is a vuln on another platform, there will be vuln there". Here your report assumes the existence of a vuln ("malicious actor were able to call this function through an approved gateway with incorrect data") to deduce a vuln. This is obviously invalid.

As per contest rules, are excluded:
Comments about the bridge being malicious.

@clesaege clesaege added the invalid This doesn't seem right label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant