Proposal cannot be executed if signers update their accounts and threshold halfway

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/cgp/governance/Multisig.sol#L30-L36

Vulnerability details

Impact

If signers update their accounts and threshold halfway into voting, then the proposal cannot be executed anymore

Proof of Concept

execute is protected under onlySigners()

    function execute(
        address target,
        bytes calldata callData,
        uint256 nativeValue
->    ) external payable onlySigners {
        _call(target, callData, nativeValue);
    }

onlySigners uses signerEpoch for the voting, and signerEpoch is updated when _rotateSigners() is called

    modifier onlySigners() {
        if (!signers.isSigner[msg.sender]) revert NotSigner();

        bytes32 topic = keccak256(msg.data);
->        Voting storage voting = votingPerTopic[signerEpoch][topic];

        // Check that signer has not voted, then record that they have voted.

     */
    function _rotateSigners(address[] memory newAccounts, uint256 newThreshold) internal {
        uint256 length = signers.accounts.length;

        // Clean up old signers.
        for (uint256 i; i < length; ++i) {
            signers.isSigner[signers.accounts[i]] = false;
        }

        length = newAccounts.length;

        if (newThreshold > length) revert InvalidSigners();

        if (newThreshold == 0) revert InvalidSignerThreshold();

->        ++signerEpoch;

If rotateSigners() is called inbetween votes, then signerEpoch will be updated. If the signers decides to update the newAccounts and newThreshold for signers before a certain proposal is executed, then the proposal will never be able to execute anymore because Voting storage voting = votingPerTopic[signerEpoch][topic] will have a different signerEpoch.

    function rotateSigners(address[] memory newAccounts, uint256 newThreshold) external virtual onlySigners {
        _rotateSigners(newAccounts, newThreshold);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Make sure that all proposals are executed first before signers are allowed to update their signer threshold and signer accounts.

Assessed type

Other

[0xSorryNotSorry]

This remains in the intended logic as a trade-off for the referenced point.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[c4-judge]

berndartmueller marked the issue as unsatisfactory: Invalid