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

Description:

Lack of ERC1271 Support Prevents Contract Interaction in Signature Validation

Description:

The advanceState function in the ProofOfHumanity contract uses ECDSA signature verification to validate vouches. However, it does not implement support for the ERC1271 standard, which is crucial for allowing smart contracts to provide signatures. This oversight prevents users who manage their identities through smart contract wallets (such as MultiSig contracts) from participating in the vouching process, significantly limiting the protocol's accessibility and potentially excluding a large portion of users who prioritize the security of contract-based wallets.

Issue Scenario / Overview:

A user who manages their identity through a MultiSig wallet attempts to participate in the vouching process. They generate a signature using their MultiSig wallet, but when they try to submit this signature through the advanceState function, the transaction fails. The function's signature verification process (using ecrecover) is unable to validate signatures from smart contracts, effectively barring these users from participating in the protocol.

Proof of Concept:

https://github.com/hats-finance/Proof-Of-Humanity-V2-0xef0709445d394a22704850c772a28a863bb780b0/blob/master/contracts/ProofOfHumanity.sol#L903-L985

// The problematic code is in the advanceState function
function advanceState(
    address _claimer,
    address[] calldata _vouches,
    SignatureVouch[] calldata _signatureVouches
) external {
    // ... (other code)

    while (request.vouches.length < requiredVouches) {
        if (i < nbSignatureVouches) {
            SignatureVouch memory signature = _signatureVouches[i];

            // This part only supports EOA signatures, not contract signatures
            voucherAccount = ecrecover(
                keccak256(
                    abi.encodePacked(
                        "\x19\x01",
                        DOMAIN_SEPARATOR,
                        keccak256(
                            abi.encode(IS_HUMAN_VOUCHER_TYPEHASH, _claimer, humanityId, signature.expirationTime)
                        )
                    )
                ),
                signature.v,
                signature.r,
                signature.s
            );

Revised Code File:

function advanceState(
    address _claimer,
    address[] calldata _vouches,
    SignatureVouch[] calldata _signatureVouches
) external {
    // ... (other code)

    while (request.vouches.length < requiredVouches) {
        if (i < nbSignatureVouches) {
            SignatureVouch memory signature = _signatureVouches[i];

+           // Check if the signer is a contract
+           if (isContract(voucherAccount)) {
+               // Use ERC1271 for contract signatures
+               bytes memory data = abi.encodePacked(
+                   "\x19\x01",
+                   DOMAIN_SEPARATOR,
+                   keccak256(abi.encode(IS_HUMAN_VOUCHER_TYPEHASH, _claimer, humanityId, signature.expirationTime))
+               );
+               bytes4 magicValue = IERC1271(voucherAccount).isValidSignature(data, abi.encodePacked(signature.r, signature.s, signature.v));
+               require(magicValue == 0x1626ba7e, "Invalid contract signature");
+           } else {
                // Existing code for EOA signatures
                voucherAccount = ecrecover(
                    keccak256(
                        abi.encodePacked(
                            "\x19\x01",
                            DOMAIN_SEPARATOR,
                            keccak256(
                                abi.encode(IS_HUMAN_VOUCHER_TYPEHASH, _claimer, humanityId, signature.expirationTime)
                            )
                        )
                    ),
                    signature.v,
                    signature.r,
                    signature.s
                );

}

+// Helper function to check if an address is a contract
+function isContract(address addr) internal view returns (bool) {
+    uint32 size;
+    assembly {
+        size := extcodesize(addr)
+    }
+    return (size > 0);
+}

This revised code adds support for ERC1271, allowing contract wallets to participate in the vouching process. It first checks if the signer is a contract, and if so, uses the ERC1271 isValidSignature function to validate the signature. If not, it falls back to the existing ECDSA signature verification for EOAs.

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