setCommittee function can set one member as the _committee and _signatureThreshold to one

[hats-bug-reporter[bot]]

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

Description: Description\ The intention of the protocol is to have a number of members as committee and not just one. And that's why the _committee parameter in the setCommittee and _setCommittee function is an array.

However, it is possible to set one member as the _committee and also have one signatureThreshold. This defeats the intention of the protocol.

What this means is that one member can call the receiveRequest. Whereas, this suppose to be a governance action that should involve more than one member.

1. Proof of Concept (PoC) File

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

import {Test, console2} from "forge-std/Test.sol";

contract Committee {
    uint256 public committeeId;
    mapping(bytes32 => bool) private committee;
    mapping(uint256 => uint256) public committeeSize;
    mapping(uint256 => uint256) public signatureThreshold;

    event CommitteeUpdated(uint256 _committeeId);

    error ZeroSignatureTreshold();
    error NotEnoughGuardians();
    error DuplicateCommitteeMember();

    function setCommittee(
        address[] calldata _committee,
        uint256 _signatureThreshold
    ) external /*onlyOwner*/ {
        ++committeeId;
        _setCommittee(_committee, _signatureThreshold);
        emit CommitteeUpdated(committeeId);
    }

    function _setCommittee(
        address[] calldata _committee,
        uint256 _signatureThreshold
    ) internal {
        if (_signatureThreshold == 0) revert ZeroSignatureTreshold();
        if (_committee.length < _signatureThreshold)
            revert NotEnoughGuardians();

        for (uint256 i; i < _committee.length; ++i) {
            bytes32 committeeMemberId = keccak256(
                abi.encodePacked(committeeId, _committee[i])
            );

            // avoid duplicates
            if (committee[committeeMemberId]) {
                revert DuplicateCommitteeMember();
            }

            committee[committeeMemberId] = true;
        }

        committeeSize[committeeId] = _committee.length;
        signatureThreshold[committeeId] = _signatureThreshold;
    }
}

contract CommitteeTest is Test {
    address user = vm.addr(1);
    Committee public committee;

    function setUp() public {
        committee = new Committee();
    }

    function testSignatureThresholdTooHigh() public {
        address[] memory members = new address[](1);
        members[0] = address(0x1);

        vm.prank(user); // Impersonate the contract owner
       // vm.expectRevert();
        committee.setCommittee(members, 1);
    }
}

Output:

forge test -vvvv [⠒] Compiling... [⠆] Compiling 1 files with 0.8.22 [⠔] Solc 0.8.22 finished in 1.62s Compiler run successful!

Running 1 test for test/A.t.sol:CommitteeTest [PASS] testSignatureThresholdTooHigh() (gas: 101975) Traces: [101975] CommitteeTest::testSignatureThresholdTooHigh() ├─ [0] VM::prank(0x7E5F4552091A69125d5DfCb7b8C2659029395Bdf) │ └─ ← () ├─ [91347] Committee::setCommittee([0x0000000000000000000000000000000000000001], 1) │ ├─ emit CommitteeUpdated(_committeeId: 1) │ └─ ← () └─ ← ()

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 106.80ms

Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

2. Revised Code File (Optional)

[krzysztofziobro]

Setting wrong threshold value is considered to be an owner error: out of scope.