Submission Details
Impact:
Likelihood:
Timelock can be bypassed
Author Revealed upon completion
Root + Impact
Description
-
The timelock should provide a delay for high impact transactions
-
The problem is that the
_getTimelockDelay()function only considers thevalueparameter (ETH amount) which ignores thedatapayload andtoaddress. This allows signers to execute high impact contract calls with 0 ETH value immediately
function _getTimelockDelay(uint256 value) internal pure returns (uint256) {
uint256 sevenDaysTimeDelayAmount = 100 ether;
uint256 twoDaysTimeDelayAmount = 10 ether;
uint256 oneDayTimeDelayAmount = 1 ether;
@> // Only checks ETH value ignores data payload and contract interactions
if (value >= sevenDaysTimeDelayAmount) {
return SEVEN_DAYS_TIME_DELAY;
} else if (value >= twoDaysTimeDelayAmount) {
return TWO_DAYS_TIME_DELAY;
} else if (value >= oneDayTimeDelayAmount) {
return ONE_DAY_TIME_DELAY;
} else {
@> return NO_TIME_DELAY; // Any value < 1 ETH executes immediately
}
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// ...
@> uint256 requiredDelay = _getTimelockDelay(txn.value); // Only considers ETH
uint256 executionTime = txn.proposedAt + requiredDelay;
// ...
@> (bool success,) = payable(txn.to).call{value: txn.value}(txn.data); // Can call any contract
}
Risk
Likelihood:
-
Really straightforward, no complex mechanism required
-
Reason 2
Impact:
-
Full bypass of timelock protection for non ETH assets
-
Protocol operations executed without safety delay
Proof of Concept
// SPDX-License-Identifier: MIT
import {MultiSigTimelock} from "./MultiSigTimelock.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
contract MockToken is ERC20 {
constructor() ERC20("Mock", "MCK") {
_mint(msg.sender, 1000000 * 10**18);
}
}
contract TimelockBypassExploit {
function testBypassTimelockWithTokenTransfer() external {
MultiSigTimelock wallet = new MultiSigTimelock();
MockToken token = new MockToken();
address signer1 = address(0x1);
address signer2 = address(0x2);
address attacker = address(0x9999);
wallet.grantSigningRole(signer1);
wallet.grantSigningRole(signer2);
// Transfer 1M tokens to wallet
token.transfer(address(wallet), 1000000 * 10**18);
// Propose token transfer with value = 0
bytes memory data = abi.encodeWithSignature(
"transfer(address,uint256)",
attacker,
1000000 * 10**18
);
uint256 txId = wallet.proposeTransaction(address(token), 0, data);
// Get 3 confirmations
wallet.confirmTransaction(txId);
vm.prank(signer1);
wallet.confirmTransaction(txId);
vm.prank(signer2);
wallet.confirmTransaction(txId);
wallet.executeTransaction(txId);
assert(token.balanceOf(attacker) == 1000000 * 10**18);
assert(token.balanceOf(address(wallet)) == 0);
}
}
Recommended Mitigation
function _getTimelockDelay(uint256 value) internal pure returns (uint256) {
uint256 sevenDaysTimeDelayAmount = 100 ether;
uint256 twoDaysTimeDelayAmount = 10 ether;
uint256 oneDayTimeDelayAmount = 1 ether;
if (value >= sevenDaysTimeDelayAmount) {
return SEVEN_DAYS_TIME_DELAY;
} else if (value >= twoDaysTimeDelayAmount) {
return TWO_DAYS_TIME_DELAY;
} else if (value >= oneDayTimeDelayAmount) {
return ONE_DAY_TIME_DELAY;
} else {
return NO_TIME_DELAY;
}
}
+ function _getTimelockDelayWithData(uint256 value, address to, bytes memory data) internal view returns (uint256) {
+ // If calling a contract with data, apply minimum 1-day delay
+ if (data.length > 0 || to.code.length > 0) {
+ // For contract calls, use at least ONE_DAY_TIME_DELAY
+ uint256 ethDelay = _getTimelockDelay(value);
+ return ethDelay > ONE_DAY_TIME_DELAY ? ethDelay : ONE_DAY_TIME_DELAY;
+ }
+
+ // For plain ETH transfers to EOAs, use value-based delay
+ return _getTimelockDelay(value);
+ }
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// CHECKS
// 1. Check if enough confirmations
if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
// 2. Check if timelock period has passed
- uint256 requiredDelay = _getTimelockDelay(txn.value);
+ uint256 requiredDelay = _getTimelockDelayWithData(txn.value, txn.to, txn.data);
uint256 executionTime = txn.proposedAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
// ... rest of function
}
Rewards breakdown
nSLOC
205This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.