Description
-
A value‑based timelock should prevent large outflows from being executed quickly, ensuring signers have a long review window before high‑value transfers leave the wallet.
-
The contract computes delay per transaction using only txn.value. There is no aggregation across pending/executed transactions within a period. An attacker or careless operator can split a ≥100 ETH payment into many smaller proposals (e.g., 100×1 ETH). Each smaller transaction receives a short delay (1 day), allowing rapid serial execution that collectively bypasses the intended 7‑day window for ≥100 ETH outflows.
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
uint256 requiredDelay = _getTimelockDelay(txn.value);
uint256 executionTime = txn.proposedAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) { revert MultiSigTimelock__ExecutionFailed(); }
emit TransactionExecuted(txnId, txn.to, txn.value);
}
function _getTimelockDelay(uint256 value) internal pure returns (uint256) {
...
}
Risk
Likelihood: Medium
-
Treasury operations often pay large sums; splitting into many 1–10 ETH transfers is trivial and common in scripting or manual batching.
-
This will occur whenever payers prefer faster settlement and can restructure payments into smaller chunks that fit shorter delay tiers.
Impact: High
-
Policy bypass: The effective 7‑day review window for ≥100 ETH is defeated; the same total can exit in ~1 day through multiple smaller transfers.
-
Governance erosion & monitoring complexity: Observers may think the timelock protects large outflows; in reality, cumulative outflows can drain quickly, undermining risk controls.
Proof of Concept
function testTimelockBypassThroughSplitting() public {
address RECIPIENT = makeAddr("recipient");
vm.deal(address(multiSigTimelock), 100 ether);
console2.log("MultiSigTimelock initial balance:", address(multiSigTimelock).balance);
console2.log("Recipient initial balance:", RECIPIENT.balance);
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
uint256 bigTxnId = multiSigTimelock.proposeTransaction(RECIPIENT, 100 ether, hex"");
multiSigTimelock.confirmTransaction(bigTxnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(bigTxnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(bigTxnId);
vm.warp(block.timestamp + 1 days);
vm.expectRevert();
multiSigTimelock.executeTransaction(bigTxnId);
uint256[] memory smallIds = new uint256[](100);
for (uint256 i = 0; i < 100; i++) {
smallIds[i] = multiSigTimelock.proposeTransaction(RECIPIENT, 1 ether, hex"");
multiSigTimelock.confirmTransaction(smallIds[i]);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(smallIds[i]);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(smallIds[i]);
}
vm.warp(block.timestamp + 1 days);
for (uint256 i = 0; i < 100; i++) {
multiSigTimelock.executeTransaction(smallIds[i]);
}
assertEq(RECIPIENT.balance, 100 ether);
console2.log("Recipient final balance after splitting strategy:", RECIPIENT.balance);
console2.log("MultiSigTimelock final balance after splitting strategy:", address(multiSigTimelock).balance);
}
Output:
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/unit/MultiSigTimelockTest.t.sol:MultiSigTimeLockTest
[PASS] testTimelockBypassThroughSplitting() (gas: 22144974)
Logs:
MultiSigTimelock initial balance: 100000000000000000000
Recipient initial balance: 0
Recipient final balance after splitting strategy: 100000000000000000000
MultiSigTimelock final balance after splitting strategy: 0
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 19.72ms (15.89ms CPU time)
Ran 1 test suite in 21.03ms (19.72ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
-
Aggregate‑aware timelock (global rolling window)
-
Scale the timelock based on cumulative queued value in a rolling period (e.g., 7 days), not just single‑tx value.
+ // Track total 'queued but not executed' ETH over a rolling window
+ struct Bucket { uint256 amount; uint256 windowStart; }
+ Bucket private s_globalOutflow; // simple single-bucket example
+ // Helper to compute rolling sum (naive single-window illustration)
+ function _pendingOutflowWithinWindow(uint256 nowTs, uint256 window) internal view returns (uint256) {
+ if (nowTs > s_globalOutflow.windowStart + window) return 0;
+ return s_globalOutflow.amount;
+ }
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
});
s_transactionCount++;
+ // Update global outflow bucket (reset window if expired)
+ uint256 W = 7 days;
+ if (block.timestamp > s_globalOutflow.windowStart + W) {
+ s_globalOutflow.windowStart = block.timestamp;
+ s_globalOutflow.amount = 0;
+ }
+ s_globalOutflow.amount += value;
emit TransactionProposed(transactionId, to, value);
return transactionId;
}
function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
- uint256 requiredDelay = _getTimelockDelay(txn.value);
+ // Base delay from single tx value
+ uint256 requiredDelay = _getTimelockDelay(txn.value);
+ // Escalate delay if cumulative queued outflow in window crosses thresholds
+ uint256 W = 7 days;
+ uint256 cumulative = _pendingOutflowWithinWindow(block.timestamp, W);
+ if (cumulative >= 100 ether && requiredDelay < SEVEN_DAYS_TIME_DELAY) {
+ requiredDelay = SEVEN_DAYS_TIME_DELAY;
+ } else if (cumulative >= 10 ether && requiredDelay < TWO_DAYS_TIME_DELAY) {
+ requiredDelay = TWO_DAYS_TIME_DELAY;
+ } else if (cumulative >= 1 ether && requiredDelay < ONE_DAY_TIME_DELAY) {
+ requiredDelay = ONE_DAY_TIME_DELAY;
+ }
uint256 executionTime = txn.proposedAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
if (txn.value > address(this).balance) {
revert MultiSigTimelock__InsufficientBalance(address(this).balance);
}
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) { revert MultiSigTimelock__ExecutionFailed(); }
+ // On success, reduce queued outflow
+ if (block.timestamp <= s_globalOutflow.windowStart + W && s_globalOutflow.amount >= txn.value) {
+ s_globalOutflow.amount -= txn.value;
+ }
emit TransactionExecuted(txnId, txn.to, txn.value);
}
