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

Proposal Double Execution Vulnerability

mrudenko

Summary

The _executeProposal function in the Governance contract lacks a check to ensure a proposal hasn't already been executed. This allows for a proposal to be executed multiple times, potentially leading to unintended consequences and state corruption.

Vulnerability Details

The _executeProposal function in contracts/core/governance/proposals/Governance.sol is responsible for executing a proposal after it has been successfully voted on and the timelock delay has passed. The function retrieves the ProposalCore struct and calls the _timelock.executeBatch function. After the timelock execution, the proposal.executed flag is set to true. However, there is no check performed at the beginning of the function to verify that proposal.executed is false before proceeding with the execution.

function _executeProposal(uint256 proposalId) internal {
ProposalCore storage proposal = _proposals[proposalId];
bytes32 salt = proposal.descriptionHash;
bytes32 id = _timelock.hashOperationBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
salt
);
​
// Check if ready for execution
if (!_timelock.isOperationReady(id)) {
revert ProposalNotQueued(proposalId, id);
}
​
// Execute through timelock
_timelock.executeBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
salt
);
​
proposal.executed = true; //@audit no check on start if proposal has been already executed
emit ProposalExecuted(proposalId, msg.sender, block.timestamp);
}

Impact

If a proposal can be re-executed, the impact depends on the nature of the proposal itself. For example, if a proposal involves transferring tokens or updating a critical system parameter, executing it multiple times could lead to:

  • Draining of funds from a contract.

  • Incorrect system state, potentially breaking core functionality.

  • Unexpected behavior and security vulnerabilities.

Tools Used

  • Manual code review.

Recommendations

Add a check at the beginning of the _executeProposal function to ensure that the proposal has not already been executed. This can be done by checking the proposal.executed flag. If the flag is already set to true, the function should revert with an appropriate error message.

function _executeProposal(uint256 proposalId) internal {
ProposalCore storage proposal = _proposals[proposalId];
​
if (proposal.executed) {
revert ProposalAlreadyExecuted(proposalId, block.timestamp);
}
bytes32 salt = proposal.descriptionHash;
bytes32 id = _timelock.hashOperationBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
salt
);
​
// Check if ready for execution
if (!_timelock.isOperationReady(id)) {
revert ProposalNotQueued(proposalId, id);
}
​
// Execute through timelock
_timelock.executeBatch(
proposal.targets,
proposal.values,
proposal.calldatas,
bytes32(0),
salt
);
​
proposal.executed = true;
emit ProposalExecuted(proposalId, msg.sender, block.timestamp);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
5 months ago
inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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