Submission Details
Impact:
Likelihood:
Timelock delay begins at proposal instead of approval
Author Revealed upon completion
Root + Impact
Description
-
Usually a timelock should provide a reaction time after a transaction is approved which gives stakeholders time to react to malicious transactions before execution
-
The problem is that the
proposedAttimestamp is set when the transaction is created and the timelock expiration is calculated asproposedAt + delay. This allows a proposer to create a transaction wait for the entire timelock period to pass with 0 confirmations then rapidly collect 3 confirmations and execute them immediately
function _proposeTransaction(address to, uint256 value, bytes memory data) internal returns (uint256) {
uint256 transactionId = s_transactionCount;
s_transactions[transactionId] = Transaction({
to: to,
value: value,
data: data,
confirmations: 0,
@> proposedAt: block.timestamp,
executed: false
});
// ...
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// ...
uint256 requiredDelay = _getTimelockDelay(txn.value);
@> uint256 executionTime = txn.proposedAt + requiredDelay; // Uses proposal time not approval time
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
}
Risk
Likelihood:
-
No complex setup required
-
Reason 2
Impact:
-
Stakeholders have no time to detect malicious approved transactions
-
Emergency response becomes useless
Proof of Concept
// SPDX-License-Identifier: MIT
import {MultiSigTimelock} from "./MultiSigTimelock.sol";
contract TimelockPreAgingExploit {
function testPreAgeTransaction() external {
MultiSigTimelock wallet = new MultiSigTimelock();
payable(address(wallet)).transfer(100 ether);
address signer1 = address(0x1);
address signer2 = address(0x2);
wallet.grantSigningRole(signer1);
wallet.grantSigningRole(signer2);
// Propose 100 ETH transaction
uint256 txId = wallet.proposeTransaction(address(0x9999), 100 ether, "");
vm.warp(block.timestamp + 7 days + 1);
// Now quickly collect confirmations
wallet.confirmTransaction(txId);
vm.prank(signer1);
wallet.confirmTransaction(txId);
vm.prank(signer2);
wallet.confirmTransaction(txId);
wallet.executeTransaction(txId);
assert(address(0x9999).balance == 100 ether);
}
}
Recommended Mitigation
struct Transaction {
address to;
uint256 value;
bytes data;
uint256 confirmations;
uint256 proposedAt;
+ uint256 approvedAt; // Track when threshold was reached
bool executed;
}
function _confirmTransaction(uint256 txnId) internal {
if (s_signatures[txnId][msg.sender]) {
revert MultiSigTimeLock__UserAlreadySigned();
}
s_signatures[txnId][msg.sender] = true;
// Increase counter
s_transactions[txnId].confirmations++;
+
+ // Set approvedAt timestamp when threshold is first reached
+ if (s_transactions[txnId].confirmations == REQUIRED_CONFIRMATIONS &&
+ s_transactions[txnId].approvedAt == 0) {
+ s_transactions[txnId].approvedAt = block.timestamp;
+ }
emit TransactionConfirmed(txnId, msg.sender);
}
function _revokeConfirmation(uint256 txnId) internal {
if (!s_signatures[txnId][msg.sender]) {
revert MultiSigTimeLock__UserHasNotSigned();
}
// Remove their signature
s_signatures[txnId][msg.sender] = false;
// Decrease counter
s_transactions[txnId].confirmations--;
+
+ // Clear approvedAt if confirmations drop below threshold
+ if (s_transactions[txnId].confirmations < REQUIRED_CONFIRMATIONS) {
+ s_transactions[txnId].approvedAt = 0;
+ }
emit TransactionRevoked(txnId, msg.sender);
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// CHECKS
if (txn.confirmations < REQUIRED_CONFIRMATIONS) {
revert MultiSigTimelock__InsufficientConfirmations(REQUIRED_CONFIRMATIONS, txn.confirmations);
}
+
+ // Require approvedAt to be set
+ if (txn.approvedAt == 0) {
+ revert("Transaction not approved");
+ }
+
// Check if timelock period has passed since approval
uint256 requiredDelay = _getTimelockDelay(txn.value);
- uint256 executionTime = txn.proposedAt + requiredDelay;
+ uint256 executionTime = txn.approvedAt + 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.