hats-finance / Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0

Proof of Humanity Protocol v2
2 stars 1 forks source link

Solidity Version Susceptible to .selector-related Optimizer Bug #68

Open hats-bug-reporter[bot] opened 2 weeks ago

hats-bug-reporter[bot] commented 2 weeks ago

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

Description:

Solidity Version Susceptible to .selector-related Optimizer Bug

Description

The CrossChainProofOfHumanity contract is using Solidity version 0.8.20, which is susceptible to a .selector-related optimizer bug. This bug can potentially lead to incorrect code generation when using .selector in certain contexts, particularly when a function call is used instead of a direct contract name for selector lookup. While the impact is generally low and affects uncommon code patterns, it's important to address this issue to ensure the contract functions correctly and securely in all scenarios.

Attack Scenario

In the current implementation, the .selector is used in the updateHumanity and transferHumanity functions when encoding function calls for cross-chain communication. While the usage in this contract is likely safe, there's a small risk that the optimizer could generate incorrect code, potentially leading to unexpected behavior in edge cases or future modifications of the contract.

Proof of Concept

The vulnerable code is present in two locations:

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

// In transferHumanity function
IBridgeGateway(_bridgeGateway).sendMessage(
    abi.encodeWithSelector(
        ICrossChainProofOfHumanity.receiveTransfer.selector,
        msg.sender,
        humanityId,
        expirationTime,
        tHash
    )
);

Revised Code File

pragma solidity 0.8.20;
+pragma solidity 0.8.21;

import {IBridgeGateway} from "./bridge-gateways/IBridgeGateway.sol";
import {IProofOfHumanity} from "./interfaces/IProofOfHumanity.sol";
import {ICrossChainProofOfHumanity} from "./interfaces/ICrossChainProofOfHumanity.sol";

contract CrossChainProofOfHumanity is ICrossChainProofOfHumanity {
    // ... (rest of the contract code remains unchanged)
}

By updating to Solidity 0.8.21 or later, we can eliminate the risk associated with the .selector-related optimizer bug, ensuring that the contract functions as intended in all scenarios, including potential future modifications or edge cases.

clesaege commented 2 weeks ago

I think you are referring to https://soliditylang.org/blog/2023/07/19/missing-side-effects-on-selector-access-bug/ From my understanding, the use of selectors does not fit the case where a bug can arise. This is also supported by the fact that the behaviour seems to be working. If you believe there is a bug, please show this selector being incorrect.