Handle
defsec
Vulnerability details
Impact
Eth sent to Timelock will be locked in current implementation. I came across this problem while playing around with the governance contract.
Proof of Concept
- Setup the governance contracts (GovernanceAlpha, Timelock)
- Send eth to timelock contract
- Setup a proposal to send 0.1 eth out. Code snippet in ether.js below. proxy refers to GovernorAlpha.
await proxy.propose(
[signers[3].address],
[ethers.utils.parseEther("0.1")],
[""],
[ethers.BigNumber.from(0)],
"Send funds to 3rd signer"
);
- Vote and have the proposal succeed.
- Execute the proposal, the proposal number here is arbitrary.
await proxy.execute(2); // this fails
await proxy.execute(2, {value: ethers.utils.parseEther("0.1")}) // this would work
0.1 eth will be sent out, but it is sent from the msg.sender not from the timelock contract.
Tools Used
Recommended Mitigation Steps
Consider implementing the following code.
function execute(uint proposalId) external {
require(state(proposalId) == ProposalState.Queued, "GovernorAlpha::execute: proposal can only be executed if it is queued");
Proposal storage proposal = proposals[proposalId];
proposal.executed = true;
for (uint i = 0; i < proposal.targets.length; i++) {
timelock.executeTransaction(proposal.targets[i], proposal.values[i], proposal.signatures[i], proposal.calldatas[i], proposal.eta);
}
emit ProposalExecuted(proposalId);
}
## Reference
https://github.com/compound-finance/compound-protocol/pull/177/files
Handle
defsec
Vulnerability details
Impact
Eth sent to Timelock will be locked in current implementation. I came across this problem while playing around with the governance contract.
Proof of Concept
Tools Used
Recommended Mitigation Steps
Consider implementing the following code.
## Reference
https://github.com/compound-finance/compound-protocol/pull/177/files