MultiSig Timelock
First Flight #55
Beginner FriendlyWallet
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: high
Invalid

Missing Accounting Logic Leads to Gas Waste and Zombie Transactions

mafa

Missing Accounting Logic Leads to Gas Waste and Zombie Transactions

Description

The contract lacks an internal accounting ledger. The proposeTransaction function does not verify if the value is valid or if it exceeds the contract's available balance. Furthermore, executeTransaction only checks address(this).balance instead of verifying the actual usable funds. This allows multiple invalid transactions to be proposed, leading to significant Gas waste as signers expend resources on transactions destined to fail. Additionally, it causes management chaos, flooding the multisig queue with unexecutable "zombie transactions.

function _executeTransaction(uint256 txnId) internal {
...
​
@> if (txn.value > address(this).balance) {
@> revert MultiSigTimelock__InsufficientBalance(address(this).balance);
}

Risk

Likelihood: The contract fails to verify the actual available balance, allowing owners to create transactions that are destined to fail due to the lack of fund tracking.

Impact:

  1. Gas Waste: Signers lose non-refundable gas fees confirming transactions destined to fail.

  2. Operational DoS: The queue is cluttered with "zombie transactions," hindering the identification of valid operations.

  3. Fund Blocking: Pending high-value proposals may obscure or delay urgent, executable transactions.

Proof of Concept

The owner created two transactions with a total value exceeding the contract balance. Due to the lack of balance checks, both were successfully proposed and confirmed. However, only the first transaction could execute; the second one inevitably reverted due to insufficient funds. As a result, a transaction destined to fail wasted 199,680 Gas.

place the following code in MultiSigTimelockTest.t.sol:

function testNoAccoutingSystem() public grantSigningRoles {
vm.deal(address(multiSigTimelock), 10 ether);
​
// owner propose and confirm 2 transaction
vm.startPrank(OWNER);
uint256 trxId_1 = multiSigTimelock.proposeTransaction(SIGNER_TWO, 8 ether, "");
multiSigTimelock.confirmTransaction(trxId_1);
​
uint256 gasBefore1 = gasleft();
uint256 trxId_2 = multiSigTimelock.proposeTransaction(SIGNER_TWO, 8 ether, "");
multiSigTimelock.confirmTransaction(trxId_2);
uint256 gasAfter1 = gasleft();
uint256 gasWaste1 = gasBefore1 - gasAfter1;
​
vm.stopPrank();
​
// 2 other signer confirm the transaction
vm.startPrank(SIGNER_TWO);
multiSigTimelock.confirmTransaction(trxId_1);
​
uint256 gasBefore2 = gasleft();
multiSigTimelock.confirmTransaction(trxId_2);
uint256 gasAfter2 = gasleft();
uint256 gasWaste2 = gasBefore2 - gasAfter2;
vm.stopPrank();
​
vm.startPrank(SIGNER_THREE);
multiSigTimelock.confirmTransaction(trxId_1);
​
uint256 gasBefore3 = gasleft();
multiSigTimelock.confirmTransaction(trxId_2);
uint256 gasAfter3 = gasleft();
uint256 gasWaste3 = gasBefore3 - gasAfter3;
vm.stopPrank();
​
// pass the locked time
vm.warp(block.timestamp + 1 days);
​
// execute the two transaction
vm.startPrank(OWNER);
multiSigTimelock.executeTransaction(trxId_1);
// trx 1 will success
MultiSigTimelock.Transaction memory trx1 = multiSigTimelock.getTransaction(trxId_1);
assert(trx1.confirmations >= 3);
assert(trx1.executed);
// trx2 will fail
vm.expectRevert();
uint256 gasBefore4 = gasleft();
multiSigTimelock.executeTransaction(trxId_2);
uint256 gasAfter4 = gasleft();
uint256 gasWaste4 = gasBefore4 - gasAfter4;
​
vm.stopPrank();
// trx2 is valid and can get all the confirmation, but will revert
MultiSigTimelock.Transaction memory trx2 = multiSigTimelock.getTransaction(trxId_2);
assert(trx2.confirmations >= 3);
assert(!trx2.executed);
// total gas spend for the trx2
uint256 gasWasteTotal = gasWaste1 + gasWaste2 + gasWaste3 + gasWaste4;
​
console2.log("gas waste 1:", gasWaste1);
console2.log("gas waste 2:", gasWaste2);
console2.log("gas waste 3:", gasWaste3);
console2.log("gas waste 4:", gasWaste4);
console2.log("gas waste total:", gasWasteTotal);
}
Logs:
gas waste 1: 132994
gas waste 2: 30128
gas waste 3: 30128
gas waste 4: 6430
gas waste total: 199680
​

Recommended Mitigation

Implement a state variable s_totalPendingAmount to track funds committed to active proposals.

+ uint256 private s_totalPendingAmount;
​
function _proposeTransaction(address to, uint256 value, bytes memory data) internal returns (uint256) {
​
// check value is valid
+ if (value > address(this).balance - s_totalPendingAmount){
+ revert}
.....
// update pending value
+ s_totalPendingAmount += value
}
​
function _executeTransaction(uint256 txnId) internal {
​
....
// update pending value
+ s_totalPendingAmount -= value
(bool success,) = payable(txn.to).call{value: txn.value}(txn.data);
if (!success) {
revert MultiSigTimelock__ExecutionFailed();
}
}
​
​
Updates

Lead Judging Commences

kelechikizito Lead Judge 4 days ago
Submission Judgement Published
Invalidated
Reason: Too generic

Appeal created

mafa Submitter
4 days ago
kelechikizito Lead Judge
3 days ago
kelechikizito Lead Judge 2 days ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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

Give us feedback!