DatingDapp
First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Insufficient Balance Check in submitTransaction Leads to Transaction Execution Failure

howiecht

Summary

The MultiSig::submitTransaction() function lacks a crucial check to ensure that the specified _value for a transaction is not greater than the contract's current balance. This oversight can lead to the executeTransaction() function failing, even if all required owners approve the transaction.

Vulnerability Details

The MultiSig contract's submitTransaction() function currently validates that the input _value is not zero. However, it fails to verify whether _value exceeds the contract's available balance. Consequently, a unintentional user could submit a transaction with a _value larger than the contract's funds. Although all required owners might subsequently approve the transaction, the executeTransaction() function will inevitably revert due to insufficient funds, rendering the transaction unsuccessful and potentially wasting gas.

Proof of code:

function testSubmitTransactionValueIsGreaterThanContractBalance() public {
deal(address(multiSig), 10 ether);
vm.prank(user);
multiSig.submitTransaction(address(user), 11 ether); // value is greater than contract balance
​
vm.prank(user);
multiSig.approveTransaction(0);
vm.prank(user2);
multiSig.approveTransaction(0);
​
vm.prank(user);
vm.expectRevert("Transaction failed");
multiSig.executeTransaction(0);
​
// balance remain unchanged
assertEq(address(user).balance, 0);
assertEq(address(multiSig).balance, 10 ether);
}

Impact

This leads to wasted gas fees and prevents the multi-sig wallet from functioning as intended, since funds can't be transferred if the user puts a value greater than the actual balance.

Tools Used

Recommendations

Implement a check within the submitTransaction() function to ensure the requested transfer value does not exceed the contract's current balance.

function submitTransaction(address _to, uint256 _value) external onlyOwners {
if (_to == address(0)) revert InvalidRecipient();
- if (_value == 0 ) revert InvalidAmount();
+ if (_value == 0 || _value > address(this).balance) revert InvalidAmount();
​
transactions.push(Transaction(_to, _value, false, false, false));
uint256 txId = transactions.length - 1;
emit TransactionCreated(txId, _to, _value);
}
Updates

Appeal created

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Users mistake, only impacting themselves.

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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