Lack of Type Safety in Cross-Chain Message Encoding

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xdeb98c21c483c87212467bd0f84f975dbf70ae384df677da8be460d3e7469eed Severity: low

Description:

Lack of Type Safety in Cross-Chain Message Encoding

Description

The CrossChainProofOfHumanity contract uses abi.encodeWithSelector to encode cross-chain messages in the updateHumanity and transferHumanity functions. This method of encoding is not type-safe, which could lead to potential vulnerabilities if the interface of the receiving contract changes or if there are discrepancies in argument types.

The lack of type safety means that any changes to the function signatures in the ICrossChainProofOfHumanity interface might not be caught at compile-time. This could result in runtime errors or unexpected behavior when messages are decoded and executed on the receiving chain.

While the current implementation appears to match the expected function signatures, the use of abi.encodeWithSelector introduces unnecessary risk and reduces the contract's robustness against future changes or errors.

Attack Scenario

1. The development team updates the ICrossChainProofOfHumanity interface, changing the parameter types or order in the receiveUpdate or receiveTransfer functions.
2. The CrossChainProofOfHumanity contract is not updated to reflect these changes.
3. When updateHumanity or transferHumanity is called, the message is encoded with the old parameter types/order.
4. The message is sent to the receiving chain, where it is decoded according to the new function signature.
5. This mismatch could lead to incorrect data being passed to the function, potentially resulting in:
   • Corrupt or incorrect humanity data being stored
   • Failed transactions
   • In extreme cases, potential loss of humanity status or ownership

Proof of Code

Problemaitc Code https://github.com/hats-finance/Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0/blob/master/contracts/CrossChainProofOfHumanity.sol#L231-L232

IBridgeGateway(_bridgeGateway).sendMessage(
            abi.encodeWithSelector(

Revised Code

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

[clesaege]

From my understanding of your report, there is no bugs, but you argue that if we were to change stuff we could introduce a bug. We can always introduce bugs by making changes.

As per contest rules: Only the smart contracts of the V2 are in scope. (so any hypothetical version would be out of scope)

As per the contest rules are excluded: Issues which can arise at deployment time but which didn't arise in the provided deployments (assume the contracts will always be deployed the way they were, this means that some are deployed using an upgradability proxy). In particular, the upgradable contracts use an initializer called during the deployment.