MultiSig Timelock

First Flight #55
Beginner FriendlyWallet
100 EXP
Submission Details
Impact: medium
Likelihood: medium

Timelock bypass via multiple small transactions, reducing protocol security and breaking the timelock functionality.

Author Revealed upon completion

Root + Impact

Description

Large transactions require multi-day timelocks, but the timelock can be completely bypassed by splitting large amounts into many small transactions (<1 ETH each), which have NO timelock.

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;
}
}

Risk

Likelihood:

  • Splitting the transaction value among several transactions does not directly violate any protocol terms, as the protocol only enforces timelocks on a single transaction's value.

  • Such a case can occur intentionally to avoid a longer timelock, or unintentionally, as proposed transactions can accumulate over time.

Impact:

  • Complete bypass of the timelock protection

  • High-value transactions can be executed immediately

  • Emergency response window eliminated

  • Timelock becomes security theater

Proof of Concept

Add this snipped of code to the MultiSigTimelockTest.t.sol test file.

function test_timelockBypassWithSplitedTransactions() public grantSigningRoles {
// ARRANGE
vm.deal(address(multiSigTimelock), OWNER_BALANCE_THREE);
console2.log("SPENDER_ONE balance before: ", SPENDER_ONE.balance);
// first transaction proposal below 10 ether
int256 txnId;
for (uint256 i = 0; i < 10; i++) {
vm.prank(OWNER);
uint256 txnId = multiSigTimelock.proposeTransaction(SPENDER_ONE, 0.5 ether, hex"");
// ACT
// get 3 / 5 confirmations for first transaction
vm.prank(OWNER);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(txnId);
vm.prank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(txnId);
// execute transaction immediately without timelock
vm.prank(OWNER);
multiSigTimelock.executeTransaction(txnId);
}
console2.log("SPENDER_ONE balance after: ", SPENDER_ONE.balance);
assertEq(SPENDER_ONE.balance, 5 ether, "SPENDER_ONE should have received 5 ether in total");
}

Even though the example transfers only 5 ether rather than 100, it still uncovers the opportunity to bypass any value eventually.

forge test --mt test_timelockBypassWithSplitedTransactions -vvvv

Recommended Mitigation

Enforce a minimum timelock for all transactions to implement security measures.

+ uint256 private constant MINIMUM_TIMELOCK = 1 hours;
.
.
.
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;
+ return MINIMUM_TIMELOCK;
}
}

Support

FAQs

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

Give us feedback!