Summary
Critical vulnerability in TimelockController where failed emergency actions become permanently locked due to premature state deletion. If an emergency action fails during execution, it becomes impossible to retry the action.
Vulnerability Details
In TimelockController.sol, the contract deletes emergency action state before completing execution:
function executeEmergencyAction(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external payable onlyRole(EMERGENCY_ROLE) nonReentrant {
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);
}
}
emit EmergencyActionExecuted(id);
}
This creates a permanent denial of service condition for emergency actions that fail during execution.
Impact
Emergency actions become permanently unusable if execution fails
Protocol would require redeployment to restore emergency functionality
Risk of fund loss during market stress when emergency functions are critical
No way to retry failed emergency actions
Proof of Concept
The PoC shows that
1. Once an emergency action is scheduled
2. If the execution fails
3. The action's state is deleted before completion
4. Making it impossible to retry the action
5. Effectively locking critical emergency functionality
This creates a serious vulnerability in the protocol's emergency response capabilities. The following test case demonstrates this scenario using a mock target contract that intentionally fails:
const { expect } = require("chai");
const { ethers } = require("hardhat");
describe("TimelockController Emergency Action Vulnerability", function() {
let timelock;
let mockTarget;
let owner;
let emergency;
let proposer;
beforeEach(async function() {
[owner, emergency, proposer] = await ethers.getSigners();
const MockTarget = await ethers.getContractFactory("MockTarget");
mockTarget = await MockTarget.deploy();
await mockTarget.deployed();
const TimelockController = await ethers.getContractFactory("TimelockController");
timelock = await TimelockController.deploy(
2 * 24 * 60 * 60,
[proposer.address],
[emergency.address],
owner.address
);
await timelock.deployed();
});
it("Should permanently block emergency action after failed execution", async function() {
const targets = [mockTarget.address];
const values = [0];
const data = mockTarget.interface.encodeFunctionData("failingFunction", []);
const calldatas = [data];
const ZERO_BYTES32 = ethers.constants.HashZero;
const salt = ethers.utils.formatBytes32String("salt");
const actionId = await timelock.hashOperationBatch(
targets,
values,
calldatas,
ZERO_BYTES32,
salt
);
await timelock.connect(emergency).scheduleEmergencyAction(actionId);
expect(await timelock.isScheduledEmergencyAction(actionId)).to.be.true;
await expect(
timelock.connect(emergency).executeEmergencyAction(
targets,
values,
calldatas,
ZERO_BYTES32,
salt
)
).to.be.revertedWith("MockTarget: intentional failure");
expect(await timelock.isScheduledEmergencyAction(actionId)).to.be.false;
await expect(
timelock.connect(emergency).executeEmergencyAction(
targets,
values,
calldatas,
ZERO_BYTES32,
salt
)
).to.be.revertedWith("EmergencyActionNotScheduled");
});
});
contract MockTarget {
function failingFunction() external pure {
revert("MockTarget: intentional failure");
}
}
Tools Used
Recommended Mitigation
Move state deletion after successful execution:
function executeEmergencyAction(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt
) external payable onlyRole(EMERGENCY_ROLE) nonReentrant {
bytes32 id = hashOperationBatch(targets, values, calldatas, predecessor, salt);
if (!_emergencyActions[id]) revert EmergencyActionNotScheduled(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);
}
}
delete _emergencyActions[id];
emit EmergencyActionExecuted(id);
}
