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

Governance cannot execute proposals that require payable function call

glightspeed2

Summary

Lack of payable modifier and msg.valuetransfer in Governance.execute function will prevent governors to execute proposals that require payable function call.

Vulnerability Details

Root Cause Analysis

Governance contract implements the core governance functionality for RAAC protocol. veRAACToken holders can create and vote on proposals. Any users can execute already passed and queued proposals via Governance.executefunction.

Governance.executefunction then will call TimelockController.executeBatch to finalize propsal execution.

The vulnerability originates from the fact that Governance.execute function lacks of payablemodifier.

@> function execute(uint256 proposalId) external override nonReentrant { // @audit lack of payable
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.executed) revert ProposalAlreadyExecuted(proposalId, block.timestamp);
ProposalState currentState = state(proposalId);

It prevents governors execute proposals that require payable function call.

POC

Scenario

  • alice creates a proposal that requires sending msg.value to ProposeHandler contract

  • voting delay (1 day) passes

  • alice votes for it

  • bob also votes for it

  • voting period (7 days) passes

  • quorum reached and the proposal voting is passed

  • proposal is queued to timelock controller

  • timelock delay (2 days) passes

  • proposal cannot be executed due to timelockController OutOfFund error

  • alice tries to send ether to timelockController

  • but transfer also fails because timelockController doesn't have any payable fallback

How to run POC

pragma solidity ^0.8.19;
​
import "../lib/forge-std/src/Test.sol";
import {Governance} from "../contracts/core/governance/proposals/Governance.sol";
import {IGovernance} from "../contracts/interfaces/core/governance/proposals/IGovernance.sol";
import {TimelockController} from "../contracts/core/governance/proposals/TimelockController.sol";
import {veRAACToken} from "../contracts/core/tokens/veRAACToken.sol";
import {RAACToken} from "../contracts/core/tokens/RAACToken.sol";
​
contract ProposeHandler {
event log_named_uint_decimal(string key, uint256 value, uint256 decimal);
​
function handle(uint256 arg) public payable {
emit log_named_uint_decimal("ProposeHandler.handle arg", arg, 0);
emit log_named_uint_decimal("PropoerHandler.handle msg.value", msg.value, 18);
}
​
function revert(uint256 arg) public payable {
emit log_named_uint_decimal("ProposeHandler.revert arg", arg, 0);
emit log_named_uint_decimal("PropoerHandler.revert msg.value", msg.value, 18);
require(false, "forced revert");
}
}
​
contract GovernanceTest is Test {
Governance governance;
TimelockController timelockController;
veRAACToken veToken;
RAACToken raacToken;
address handler;
uint256 delay = 2 days;
uint256 lockDuartion = 365 days * 4;
​
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address eve = makeAddr("eve");
​
function setUp() external {
raacToken = new RAACToken(address(this), 0, 0);
raacToken.setMinter(address(this));
veToken = new veRAACToken(address(raacToken));
address[] memory addresses = new address[](1);
addresses[0] = address(this);
timelockController = new TimelockController(delay, addresses, addresses, address(this));
governance = new Governance(address(veToken), address(timelockController));
timelockController.grantRole(timelockController.PROPOSER_ROLE(), address(governance));
timelockController.grantRole(timelockController.EXECUTOR_ROLE(), address(governance));
timelockController.grantRole(timelockController.CANCELLER_ROLE(), address(governance));
handler = address(new ProposeHandler());
}
​
function testPayableFunctionCall() external {
_dealVeToken(alice, 100000e18);
_dealVeToken(bob, 50000e18);
// alice creates a proposal
uint256 proposalId = _propose(alice, ProposeHandler.handle.selector);
// voting delay passes
skip(1 days);
// alice and bob votes for support
_castVote(bob, proposalId, true);
_castVote(alice, proposalId, true);
// voting period passes
skip(7 days);
// proposal is passed
require(governance.state(proposalId) == IGovernance.ProposalState.Succeeded, "vote not passed");
// queue proposal
governance.execute(proposalId);
// proposal is queued
require(governance.state(proposalId) == IGovernance.ProposalState.Queued, "proposal not queued");
// timelock delay passes
skip(2 days);
​
// reverts due to EvmError: OutOfFunds
vm.expectRevert();
governance.execute(proposalId);
​
// alice tries to fund timelockController
deal(alice, 3 ether);
vm.startPrank(alice);
// revert because timelockController doesn't have any payable fallback
vm.expectRevert();
payable(address(timelockController)).transfer(3 ether);
vm.stopPrank();
}
​
function _propose(address proposer, bytes4 selector) internal returns (uint256 proposalId) {
address[] memory targets = new address[](2);
targets[0] = handler;
targets[1] = handler;
uint256[] memory values = new uint256[](2);
values[0] = 1 ether;
values[1] = 2 ether;
bytes[] memory calldatas = new bytes[](2);
calldatas[0] = abi.encodeWithSelector(selector, 1);
calldatas[1] = abi.encodeWithSelector(selector, 2);
vm.startPrank(proposer);
proposalId =
governance.propose(targets, values, calldatas, "test propose", IGovernance.ProposalType.EmissionChange);
vm.stopPrank();
}
​
function _dealVeToken(address account, uint256 amount) internal {
deal(address(raacToken), account, amount);
vm.startPrank(account);
raacToken.approve(address(veToken), amount);
veToken.lock(amount, lockDuartion);
vm.stopPrank();
}
​
function _castVote(address account, uint256 proposalId, bool support) internal {
vm.startPrank(account);
governance.castVote(proposalId, support);
vm.stopPrank();
}
}

Impact

Governance cannot execute proposals that require payable function call

Tools Used

Manual Review, Foundry

Recommendations

This can be fixed by either of the following approach:

  • Make Governance.execute function payable, and make it delegates msg.valueto TimelockController.executeBatch, or

  • Implement a payable fallback function to TimelockController

Workarounds

There are some clever workarounds for this vulnerabilty. i.e. TimelockController can be funded before proposal execution in the following ways:

  • Protocol owners can use selfdestruct to force send ether to TimelockController.

  • Admin can schedule an emergency action to ReturnEthsmart contract. This smart contract will do nothing but refunds all msg.value to msg.sender. Since TimelockController.executeEmergencyActionhas payable modifier, admin can fund eth to TimelockController in this way.

Updates

Lead Judging Commences

inallhonesty Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Governance.execute lacks payable modifier and ETH forwarding mechanism, preventing proposals with ETH transfers from being executed through TimelockController

inallhonesty Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Governance.execute lacks payable modifier and ETH forwarding mechanism, preventing proposals with ETH transfers from being executed through TimelockController

Support

FAQs

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