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

Premature Execution Flag Causes Fund Lockup

teoslaf

Summary

The executeTransaction function incorrectly marks a transaction as executed before ensuring that the transaction was successfully processed. If the transaction fails, it remains permanently marked as executed, preventing any future attempts to execute it again.

Vulnerability Details

The function contains the following flawed logic:

txn.executed = true;
(bool success, ) = payable(txn.to).call{value: txn.value}("");
require(success, "Transaction failed");

  • The transaction is marked as executed before attempting execution.

  • If the .call{value: txn.value}("") fails (due to out-of-gas, recipient rejecting Ether, or other failures), the transaction remains in a "failed but executed" state.

  • Since txn.executed = true, it can never be retried, resulting in a locked transaction.

Impact

  • Funds may become stuck: If the transaction fails, the contract prevents retrying it, meaning owners cannot resend the funds.

  • Multi-sig execution failure: The contract requires approvals from multiple owners, but a failure in execution means the intended recipient never receives the funds, yet the transaction is considered "processed."

  • Loss of funds usability: If a transaction was meant to transfer critical funds (e.g., payroll, treasury management), its failure can block further execution attempts, disrupting operations.

Tools Used

Manual code review

Recommendations

Modify the function to mark txn.executed = true; only after ensuring the transaction succeeds.

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");
​
// Execute the transaction first
(bool success, ) = payable(txn.to).call{value: txn.value}("");
​
// Only mark executed if successful
require(success, "Transaction failed");
txn.executed = true;
​
emit TransactionExecuted(_txId, txn.to, txn.value);
}
Updates

Appeal created

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood 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.