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

Lack of Input Validation in `MultiSigWallet::submitTransaction` allows submitting a transaction greater than Contract's balance

roccomania

Summary

The MultiSigWallet::submitTransaction function does not validate whether the requested transaction value is within the contract's balance.

Vulnerability Details

The function checks if _value is zero but does not verify if the contract has sufficient funds to execute the transaction. This could lead to transactions being submitted that cannot be executed.

Proof of Concept

Consider this scenario

  • User submits a transaction of value greater than the contract balance

  • The proposal passes and it got approved by both users

  • The proposal becomes inexecutable since it is now greater than contract balance.

Consider adding this test fuction to the SoulboundProfileNFTTest.t.sol test file. Make sure to import MultiSigWallet. -- import { MultiSigWallet } from "../src/MultiSig.sol";

function testSubmitInvalidMultisigTxn() public {
MultiSigWallet multiSigWallet = new MultiSigWallet(user, user2);
vm.deal(address(multiSigWallet), 10 ether);
​
vm.startPrank(user);
multiSigWallet.submitTransaction(user, 50 ether);
multiSigWallet.approveTransaction(0);
vm.stopPrank();
​
vm.startPrank(user2);
multiSigWallet.approveTransaction(0);
//Expect revert
vm.expectRevert("Transaction failed");
multiSigWallet.executeTransaction(0);
vm.stopPrank();
​
}

Impact

  • Wasted gas on transactions that cannot be executed.

  • Potential confusion for users.

Tools Used

  • Manual code review.

Recommendations

Add a check to ensure the requested value does not exceed the contract's balance:

function submitTransaction(address _to, uint256 _value) external onlyOwners {
if (_to == address(0)) revert InvalidRecipient();
if (_value == 0) revert InvalidAmount();
if (_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 6 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.