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

Missing ETH Value Validation in Governance Proposal Execution

waydou

Summary

The Governance contract executes proposals through the Timelock contract without forwarding required ETH, and the Timelock contract fails to validate that the received ETH (msg.value) matches the sum of values to be sent in the batched operations. This could lead to failed proposal executions or stuck ETH.

Vulnerability Details

First in the governance contract:

function _executeProposal(uint256 proposalId) internal {
ProposalCore storage proposal = _proposals[proposalId];
// ...
_timelock.executeBatch( // @audit No msg.value passed
proposal.targets,
proposal.values, // Array of ETH amounts to send
proposal.calldatas,
bytes32(0),
salt
);
}

Then in the Timelock contract:

function executeBatch(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external override payable nonReentrant onlyRole(EXECUTOR_ROLE) {
// @audit No validation of msg.value vs sum(values)
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);
}
}
}

Impact

  1. Governance proposals that require ETH transfers will always fail because no ETH is forwarded

  2. The Timelock contract could receive more ETH than needed for the operations, which would be stuck

  3. Operations could fail midway through batch execution due to insufficient ETH, leaving the system in a partially executed state

Tools Used

Manual review

Recommendations

Add ETH validation in Timelock's executeBatch:

function executeBatch(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external payable nonReentrant onlyRole(EXECUTOR_ROLE) {
uint256 totalValue;
for(uint256 i = 0; i < values.length; i++) {
totalValue += values[i];
}
require(msg.value == totalValue, "Incorrect ETH value sent");
// Rest of the function...
}

Modify Governance's _executeProposal to forward required ETH:

function _executeProposal(uint256 proposalId) internal {
ProposalCore storage proposal = _proposals[proposalId];
uint256 totalValue;
for(uint256 i = 0; i < proposal.values.length; i++) {
totalValue += proposal.values[i];
}
_timelock.executeBatch{value: totalValue}(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
proposal.descriptionHash
);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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