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

Missing Payable Modifier in execute Function Prevents Proposal Execution

frontrunner

Summary

The execute function in the governance contract is responsible for executing proposals by calling the _executeProposal function, which in turn calls executeBatch on the TimeLockController contract. However, the execute function is not marked as payable, preventing it from sending Ether required by executeBatch. Since both the governance and TimeLockController contracts lack any alternative mechanism to receive Ether, execution will always fail when proposals require native token transfers.

Vulnerability Details

Affected Function:

function execute(uint256 proposalId) external override nonReentrant {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.executed) revert ProposalAlreadyExecuted(proposalId, block.timestamp);
ProposalState currentState = state(proposalId);
// Check if the proposal is in the correct state for execution
if (currentState == ProposalState.Succeeded) {
// Queue the proposal
_queueProposal(proposalId);
} else if (currentState == ProposalState.Queued) {
// Execute the queued proposal
_executeProposal(proposalId);
} else {
// If not in Succeeded or Queued state, revert
revert InvalidProposalState(
proposalId,
currentState,
currentState == ProposalState.Active ? ProposalState.Succeeded : ProposalState.Queued,
"Invalid state for execution"
);
}
}

Root Cause Analysis

  1. The executeBatch function in TimeLockController is marked as payable, meaning it expects Ether when executing transactions that require native token transfers.

  2. The governance contract’s execute function is not marked as payable, preventing users from sending Ether along with the execution transaction.

  3. Neither the Governance nor TimeLockController contracts have a receive or fallback function to accept Ether, making it impossible to fund executeBatch indirectly.

  4. As a result, proposals that involve transferring Ether will always revert due to insufficient balance in the TimeLockController contract.

Affected executeBatch Function:

function executeBatch(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external override payable nonReentrant onlyRole(EXECUTOR_ROLE) {
bytes32 id = hashOperationBatch(targets, values, calldatas, predecessor, salt);
// Check operation status
Operation storage op = _operations[id];
if (op.timestamp == 0) revert OperationNotFound(id);
if (op.executed) revert OperationAlreadyExecuted(id);
// Check timing conditions
if (block.timestamp < op.timestamp) revert OperationNotReady(id);
if (block.timestamp > op.timestamp + GRACE_PERIOD) revert OperationExpired(id);
// Check predecessor if specified
if (predecessor != bytes32(0)) {
if (!isOperationDone(predecessor)) {
revert PredecessorNotExecuted(predecessor);
}
}
​
// Mark as executed before external calls
op.executed = true;
​
// Execute each call
for (uint256 i = 0; i < targets.length; i++) {
(bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
if (!success) {
revert CallReverted(id, i);
}
}
​
emit OperationExecuted(id, targets, values, calldatas, predecessor, salt);
}

Impact

Proposals requiring Ether transfers cannot be executed because the execute function does not allow users to send Ether, and the TimeLockController contract cannot receive Ether by other means. This will cause the execute function to always fail and proposal actions will not be completed.

Tools Used

  • Manual Review

Recommendations

To allow Ether transfers when executing proposals, mark the execute function as payable, enabling it to send Ether to executeBatch as needed.

Recommended Fix:

function execute(uint256 proposalId) external payable override nonReentrant {
ProposalCore storage proposal = _proposals[proposalId];
if (proposal.executed) revert ProposalAlreadyExecuted(proposalId, block.timestamp);
ProposalState currentState = state(proposalId);
// Check if the proposal is in the correct state for execution
if (currentState == ProposalState.Succeeded) {
// Queue the proposal
_queueProposal(proposalId);
} else if (currentState == ProposalState.Queued) {
// Execute the queued proposal
_executeProposal(proposalId);
} else {
// If not in Succeeded or Queued state, revert
revert InvalidProposalState(
proposalId,
currentState,
currentState == ProposalState.Active ? ProposalState.Succeeded : ProposalState.Queued,
"Invalid state for execution"
);
}
}

Adding payable allows the function to send Ether along with execution, resolving the issue.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 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 7 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.

Give us feedback!