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

`executeBatch()` and `executeEmergencyAction()` functions in `TimelockController.sol` do not properly enforce validation on `msg.value`

benterkiii

Summary

The executeBatch() and executeEmergencyAction() functions in TimelockController.sol do not properly enforce validation on msg.value in relation to the sum of values[].
This issue will lead to ETH being trapped in the contract, as any excess funds sent in msg.value beyond what is allocated in values[] are not refunded.
This results in an unintended loss.

Vulnerability Details

Both functions allow EXECUTOR_ROLE or EMERGENCY_ROLE to send ETH along with multiple governance transactions.
However, the contract does not validate that msg.value matches or exceeds the total sum of values[]. This creates a scenario where:

  1. If msg.value is greater than the sum of values[], the excess funds remain locked inside the contract indefinitely, as there is no refund mechanism,
    This is caused by the lack of a refund mechanism to send back the remaining ETH.

  2. If msg.value is less than the sum of values[], the transaction could fail or execute improperly,
    This is caused by the lack of validation logic ensuring msg.value >= sum(values[]) before execution.

Impact

This issue will cause:

  • Users executing these functions can lose ETH if they send more than required, as the excess will be trapped in the contract.

  • In cases where insufficient ETH is sent, transactions may fail.

  • The contract does not provide a way to reclaim locked funds.

Tools Used

  • Manual code review

Recommendations

To mitigate this issue implement executeBatch() and executeEmergencyAction() as follow:

  1. Enforce Validation on msg.value Before executing transactions, include a check to ensure msg.value is at least equal to the total sum of values[].

  2. Refund Excess ETH after the calls If msg.value is greater than zero, return the excess ETH to the sender.

executeBatch():

function executeBatch(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external override payable nonReentrant onlyRole(EXECUTOR_ROLE) {
+ uint256 totalValue;
+ for (uint256 i = 0; i < values.length; i++) {
+ totalValue += values[i];
+ }
+ require(msg.value >= totalValue, "Insufficient ETH sent");
​
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);
}
}
+ if (msg.value > 0) {
+ payable(msg.sender).transfer(msg.value);
+ }
​
emit OperationExecuted(id, targets, values, calldatas, predecessor, salt);
}

executeEmergencyAction():

function executeEmergencyAction(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external payable onlyRole(EMERGENCY_ROLE) nonReentrant {
+ uint256 totalValue;
+ for (uint256 i = 0; i < values.length; i++) {
+ totalValue += values[i];
+ }
+ require(msg.value >= totalValue, "Insufficient ETH sent");
​
bytes32 id = hashOperationBatch(targets, values, calldatas, predecessor, salt);
if (!_emergencyActions[id]) revert EmergencyActionNotScheduled(id);
delete _emergencyActions[id];
for (uint256 i = 0; i < targets.length; i++) {
(bool success, bytes memory returndata) = targets[i].call{value: values[i]}(calldatas[i]);
if (!success) {
if (returndata.length > 0) {
assembly {
let returndata_size := mload(returndata)
revert(add(32, returndata), returndata_size)
}
}
revert CallReverted(id, i);
}
}
+ if (msg.value > 0) {
+ payable(msg.sender).transfer(msg.value);
+ }
​
emit EmergencyActionExecuted(id);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge 7 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.

Give us feedback!