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

Risk of Gas Manipulation and DoS by a Malicious Owner

sauronsol

Summary

The executeTransaction function in the MultiSigWallet contract does not specify a gas limit for the external call, which could be exploited by a malicious owner to perform DoS attacks or manipulate gas.

Vulnerability Details

In MultiSig.sol, the executeTransaction function:

  1. Allows any owner to execute an approved transaction

  2. Imposes no gas limit on the external call

  3. Trusts both owners to act in good faith

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;
// @audit no gas limit specified
(bool success,) = payable(txn.to).call{value: txn.value}("");
require(success, "Transaction failed");
​
emit TransactionExecuted(_txId, txn.to, txn.value);
}

Impact

  1. DoS Attack : A malicious owner could create a transaction to a contract that intentionally consumes all the gas

  2. Gas Manipulation : Possibility of forcing transactions to fail by manipulating gas consumption

  3. MultiSig Blockage : In the worst case, the MultiSig could become unusable

Severity: LOW - Requires an owner to be malicious and the other owner to approve the malicious transaction

Tools Used

  • Manual code review

  • Analysis of attack scenarios

  • PoC demonstrating gas manipulation

Proof of Concept

contract MaliciousContract {
uint256 public counter;
receive() external payable {
// Infinite loop to consume all gas
while(true) {
counter++;
}
}
}
​
function testGasManipulation() public {
// Deployment of the malicious contract
MaliciousContract malicious = new MaliciousContract();
// Owner1 (malicious) creates a transaction to the malicious contract
vm.prank(owner1);
multiSig.submitTransaction(address(malicious), 1 ether);
// Both owners approve
vm.prank(owner1);
multiSig.approveTransaction(0);
vm.prank(owner2);
multiSig.approveTransaction(0);
// Execution fails due to excessive gas consumption
vm.prank(owner1);
vm.expectRevert();
multiSig.executeTransaction(0);
}

Recommendations

  1. Add an explicit gas limit:

function executeTransaction(uint256 _txId) external onlyOwners {
// ... other verifications ...
// Explicit gas limit for external call
(bool success,) = payable(txn.to).call{value: txn.value, gas: 50000}("");
require(success, "Transaction failed");
}

  1. Implement a destination code verification:

function executeTransaction(uint256 _txId) external onlyOwners {
// ... other verifications ...
// Verify if the destination is a contract
uint256 size;
address to = txn.to;
assembly {
size := extcodesize(to)
}
// If it's a contract, use a gas limit
if (size > 0) {
(bool success,) = payable(to).call{value: txn.value, gas: 50000}("");
require(success, "Transaction failed");
} else {
// If it's an EOA, simple transfer
(bool success,) = payable(to).call{value: txn.value}("");
require(success, "Transfer failed");
}
}

  1. Add a security delay for transactions to contracts:

mapping(uint256 => uint256) public transactionTimelocks;
​
function approveTransaction(uint256 _txId) external onlyOwners {
// ... usual verifications ...
// If it's a contract, add a timelock
uint256 size;
assembly {
size := extcodesize(txn.to)
}
if (size > 0) {
transactionTimelocks[_txId] = block.timestamp + 24 hours;
}
}
​
function executeTransaction(uint256 _txId) external onlyOwners {
// ... usual verifications ...
// Verify the timelock if present
if (transactionTimelocks[_txId] > 0) {
require(
block.timestamp >= transactionTimelocks[_txId],
"Transaction is timelocked"
);
}
// ... rest of the code ...
}
Updates

Appeal created

n0kto Lead Judge 5 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.