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

Hash colission could occur when calling TimelockController.sol::hashOperationBatch(), it could return an existing ID.

tamer

Summary

Another user or the same user that created their first proposal could end up creating the very same proposal with the same params.
Same targets, values, calldatas and description.
This will create the same ID after calling TimelockController.sol::hashOperationBatch().

Vulnerability Details

The TimelockController.sol::hashOperationBatch() function generates an operation ID (id) by hashing the input parameters where the parameters are provided in Governance.sol::propose().
If a propose is created twice with the same parameters, it results in the same hash ID, leading to unintended overwriting or reuse of the hash ID generated from TimelockController.sol::hashOperationBatch().

Impact

A new proposal can unintentionally overwrite an existing proposal if it has the same inputs, preventing the execution of the original proposal or a malicious actor could act badly on the proposal.

PoC of generating the same ID run test instructions:

  1. Place the provided code in test/unit/core/governance/Governance.test.js

  2. Execute the test with this command: npx hardhat test --grep "should not allow to create proposals with the same params, but it happens"

describe("POCS", () => {
it("should not allow to create proposals with the same params, but it happens", async () => {
// copy pasted the SETUP code from other tests. This is not chatGPT generated...
​
// Set up voting power for the proposer
await veToken.mock_setInitialVotingPower(
await owner.getAddress(),
ethers.parseEther("150000")
);
​
// Create a proposal
const targets = [await testTarget.getAddress()];
const values = [0];
const calldatas = [
testTarget.interface.encodeFunctionData("setValue", [42])
];
const description = "Test Proposal";
const tx = await governance.connect(owner).propose(
targets,
values,
calldatas,
description,
0 // ParameterChange
);
const receipt = await tx.wait();
const event = receipt.logs.find(
log => governance.interface.parseLog(log)?.name === 'ProposalCreated'
);
let proposalIdONE = event.args.proposalId;
​
// Advance time to voting period
await time.increase(VOTING_DELAY);
​
​
// Setup voting power
await veToken.mock_setVotingPower(await user1.getAddress(), ethers.parseEther("6000000"));
​
const startTime = await moveToNextTimeframe();
expect(await governance.state(proposalIdONE)).to.equal(ProposalState.Active);
// Cast vote
await governance.connect(user1).castVote(proposalIdONE, true);
expect(await governance.state(proposalIdONE)).to.equal(ProposalState.Active);
// Wait for voting period to end
await time.increaseTo(startTime + VOTING_PERIOD);
await network.provider.send("evm_mine");
// Verify state is Succeeded
expect(await governance.state(proposalIdONE)).to.equal(ProposalState.Succeeded);
// Queue the proposal
await governance.execute(proposalIdONE);
expect(await governance.state(proposalIdONE)).to.equal(ProposalState.Queued);
// Wait for timelock delay
const timelockDelay = await timelock.getMinDelay();
await time.increase(timelockDelay);
await network.provider.send("evm_mine");
// Execute the proposal
await governance.execute(proposalIdONE);
await logProposalState(governance, proposalIdONE);
expect(await governance.state(proposalIdONE)).to.equal(ProposalState.Executed);
​
// END OF proposalIdONE CREATION AND EXECUTION
​
​
​
​
​
​
// Set up voting power for the proposer
await veToken.mock_setInitialVotingPower(
await owner.getAddress(),
ethers.parseEther("150000")
);
​
// Create a proposal
const sameTargets = [await testTarget.getAddress()];
const sameValues = [0];
const sameCalldatas = [
testTarget.interface.encodeFunctionData("setValue", [42])
];
const txTwo = await governance.connect(owner).propose(
sameTargets,
sameValues,
sameCalldatas,
description,
0 // ParameterChange
);
const receiptTwo = await txTwo.wait();
const eventTwo = receiptTwo.logs.find(
log => governance.interface.parseLog(log)?.name === 'ProposalCreated'
);
let proposalIdTWO = eventTwo.args.proposalId;
​
// Advance time to voting period
await time.increase(VOTING_DELAY);
​
​
// START OF EXECUTION
// Setup voting power
await veToken.mock_setVotingPower(await user1.getAddress(), ethers.parseEther("6000000"));
​
const startTimeTwo = await moveToNextTimeframe();
expect(await governance.state(proposalIdTWO)).to.equal(ProposalState.Active);
// Cast vote
await governance.connect(user1).castVote(proposalIdTWO, true);
expect(await governance.state(proposalIdTWO)).to.equal(ProposalState.Active);
// Wait for voting period to end
await time.increaseTo(startTimeTwo + VOTING_PERIOD);
await network.provider.send("evm_mine");
// Verify state is Succeeded
expect(await governance.state(proposalIdTWO)).to.equal(ProposalState.Succeeded);
// Queue the proposal
// we cannot procceed from here because there is a collision with the hashOperationBatch giving the same ID
await expect(governance.execute(proposalIdTWO)).to.be.revertedWithCustomError(timelock, "OperationAlreadyScheduled");
// END OF proposalIdTWO CREATION AND EXECUTION
​
​
​
​
// check that we have different proposalIDs
expect(proposalIdTWO).to.be.gt(proposalIdONE);
​
const proposalDataIDOne = await governance.getProposalData(proposalIdONE);
const proposalDataIDTwo = await governance.getProposalData(proposalIdTWO);
// check that the params are the same
expect(proposalDataIDOne.targets[0]).to.equal(proposalDataIDTwo.targets[0]);
expect(proposalDataIDOne.values[0]).to.equal(proposalDataIDTwo.values[0]);
expect(proposalDataIDOne.calldatas[0]).to.equal(proposalDataIDTwo.calldatas[0]);
expect(proposalDataIDOne.description).to.equal(proposalDataIDTwo.description);
})
})

Tools Used

Manual

Recommendations

It would be a good idea when creating a proposal to check if the generated ID already exists.
Can be done with calling TimelockController.sol::hashOperationBatch().
This would be the first mitigation.

The better way of doing it would be to provide a really really unique param to the hashing function and the most unique thing is the proposalId since it will work like a nonce.

Updates

Lead Judging Commences

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

Governance generates non-unique timelock operation IDs for different proposals with identical parameters, allowing timelock bypass and proposal DoS attacks

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

Governance generates non-unique timelock operation IDs for different proposals with identical parameters, allowing timelock bypass and proposal DoS attacks

Support

FAQs

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