MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

Timelock bypass for non-ETH assets

Author Revealed upon completion

Description

  • The “dynamic timelock” should delay any high‑value operation from the wallet, whether it sends ETH or performs a token/privileged call via data, so signers have time to detect and stop risky transactions.

  • The timelock duration is computed only from txn.value (ETH sent). Calls with value == 0 but a powerful data payload (e.g., ERC20 transfer, approve, or an admin function on another contract) bypass the timelock entirely and can execute immediately, regardless of the economic value moved.

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
// Quorum check omitted here for brevity
​
// @> Timelock is computed solely from ETH 'value'
uint256 requiredDelay = _getTimelockDelay(txn.value);
uint256 executionTime = txn.proposedAt + requiredDelay;
if (block.timestamp < executionTime) {
revert MultiSigTimelock__TimelockHasNotExpired(executionTime);
}
​
// @> External call may transfer tokens or invoke privileged logic via 'data'
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) { revert MultiSigTimelock__ExecutionFailed(); }
}
​
function _getTimelockDelay(uint256 value) internal pure returns (uint256) {
// @> Delay tiers depend only on ETH 'value'
// <1 ETH => 0 delay; 1-10 ETH => 1 day; 10-100 ETH => 2 days; >=100 ETH => 7 days
...
}

Risk

Likelihood: High

  • Whenever the wallet holds ERC20s (treasury tokens, stables) or interacts with external contracts, signers routinely propose data‑only calls (with value == 0). These will always execute immediately after quorum, because the code assigns zero delay to < 1 ETH, including 0.

  • This occurs in normal operations (token payments, approvals, staking, governance calls) and during incidents, making timelock protection ineffective for most non‑ETH value transfers.

Impact: High

  • Governance bypass / asset loss: Large ERC20 transfers or approvals can be executed with no delay, defeating the intended safety window and enabling fast drains if signers are compromised or collude.

  • Policy mismatch: Users assume “high‑value transactions” are delayed; in practice only ETH transfers are delayed, which is misleading and dangerous for treasuries primarily holding tokens.

Proof of Concept

  • Create poc.t.sol under unit directory and copy the code below.

  • Run forge test --mp poc -vv.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console2} from "forge-std/Test.sol";
import {MultiSigTimelock} from "src/MultiSigTimelock.sol";
import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
contract MultiSigTimeLockTest is Test {
MultiSigTimelock multiSigTimelock;
​
address public immutable RECIPIENT = makeAddr("recipient");
address public immutable SIGNER_TWO = makeAddr("signer_two");
address public immutable SIGNER_THREE = makeAddr("signer_three");
​
function setUp() public {
multiSigTimelock = new MultiSigTimelock();
}
​
function testTimelockBypassForNonETHAssets() public {
// --- Arrange ---
​
// Deploy a mintable mock token and fund the wallet with a large amount
MockERC20 token = new MockERC20();
uint256 bigAmount = 1_000_000 ether;
token.mint(address(multiSigTimelock), bigAmount);
uint256 walletBalance = token.balanceOf(address(multiSigTimelock));
assertEq(walletBalance, bigAmount, "Wallet funded with tokens");
console2.log("Wallet initial token balance:", walletBalance);
​
uint256 recipientBalance = token.balanceOf(RECIPIENT);
assertEq(recipientBalance, 0, "Recipient starts with zero tokens");
console2.log("Recipient initial token balance:", recipientBalance);
​
// Prepare ERC20.transfer(recipient, bigAmount) payload with ZERO ETH value
bytes memory data = abi.encodeWithSignature("transfer(address,uint256)", RECIPIENT, bigAmount);
​
// Propose data-only transaction (value == 0 => no timelock by current logic)
uint256 txnId = multiSigTimelock.proposeTransaction(address(token), 0, data);
​
// Grant two more signers
multiSigTimelock.grantSigningRole(SIGNER_TWO);
multiSigTimelock.grantSigningRole(SIGNER_THREE);
​
// --- Act ---
// Reach quorum
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
​
// Execute immediately WITHOUT waiting (because requiredDelay == 0 for value == 0)
vm.prank(SIGNER_TWO);
multiSigTimelock.executeTransaction(txnId);
​
// --- Assert ---
recipientBalance = token.balanceOf(RECIPIENT);
assertEq(recipientBalance, bigAmount, "Tokens moved instantly");
console2.log("Recipient final token balance:", recipientBalance);
​
walletBalance = token.balanceOf(address(multiSigTimelock));
assertEq(walletBalance, 0, "Wallet drained with no delay");
console2.log("Wallet final token balance:", walletBalance);
​
}
}
​
// Minimal mock token for the test
contract MockERC20 is ERC20 {
constructor() ERC20("Mock", "MCK") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}

Output:

[⠊] Compiling...
No files changed, compilation skipped
​
Ran 1 test for test/unit/poc.t.sol:MultiSigTimeLockTest
[PASS] testTimelockBypassForNonETHAssets() (gas: 1360022)
Logs:
Wallet initial token balance: 1000000000000000000000000
Recipient initial token balance: 0
Recipient final token balance: 1000000000000000000000000
Wallet final token balance: 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.67ms (505.30µs CPU time)
​
Ran 1 test suite in 14.18ms (1.67ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

  • Make timelock apply to any nontrivial external call, not just ETH value.

  • Baseline fix (low complexity): enforce a minimum delay for calls with non‑empty data, even when value == 0:

function _executeTransaction(uint256 txnId) internal {
Transaction storage txn = s_transactions[txnId];
​
- uint256 requiredDelay = _getTimelockDelay(txn.value);
+ uint256 requiredDelay = _getTimelockDelay(txn.value);
+ // Apply at least a 1-day delay to any data-bearing call (token transfers, approvals, admin calls)
+ if (txn.data.length > 0 && requiredDelay < ONE_DAY_TIME_DELAY) {
+ requiredDelay = ONE_DAY_TIME_DELAY;
+ }
​
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);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!