hats-finance / Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d

Aleph Zero bridge to Ethereum
Apache License 2.0
0 stars 1 forks source link

It's possible not to reach the required signature threshold #53

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x8250ef1a95dbc4f53a354d78d8dafd301e751d77c34c397f64b7f92ca2f25d95 Severity: high

Description: Description\ The _setCommittee function checks for _signatureThreshold being zero with the ZeroSignatureTreshold() revert, but it doesn't check if _signatureThreshold is greater than the length of the _committee array.

This could potentially lead to a situation where the _signatureThreshold is set to a value higher than the number of committee members, making it impossible to reach the required threshold.

Foundry test is provided below.

Attachments

  1. Proof of Concept (PoC) File
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import {Test, console2} from "forge-std/Test.sol";
//import "../src/A.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[](2);
        members[0] = address(0x1);
        members[1] = address(0x2);

        vm.prank(user); // Impersonate the contract owner
        vm.expectRevert(Committee.NotEnoughGuardians.selector);
        committee.setCommittee(members, 3);
    }
}

Here's the result:

forge test [⠑] Compiling... [⠔] Compiling 1 files with 0.8.22 [⠑] Solc 0.8.22 finished in 1.68s Compiler run successful!

Running 1 test for test/A.t.sol:CommitteeTest [PASS] testSignatureThresholdTooHigh() (gas: 33876) Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 112.74ms

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

  1. Revised Code File (Optional)
ololade97 commented 5 months ago

**Please disregard this submission

krzysztofziobro commented 5 months ago

Submission assumes owner misbehavior/error: invalid