DatingDapp

Frontrunning Vulnerability in submitTransaction

Description:

The submitTransaction function directly pushes a new transaction into the transactions array, and its index (txId) is deterministically assigned as transactions.length - 1. This makes transaction IDs predictable, allowing an attacker monitoring the mempool to frontrun the submission and manipulate transaction approvals or execution in their favor.

For example, a malicious owner could submit a competing transaction with a slightly higher gas fee, ensuring their transaction is mined first. This could lead to unexpected approvals or block the intended transaction.

Impact:

• Transaction Manipulation: Malicious owners can front-run legitimate transactions, affecting execution order and approvals.

• Denial of Service (DoS): Attackers can spam transactions, making it difficult to execute an intended transaction.

• Potential Exploits: If a transaction involves time-sensitive operations (e.g., arbitrage, flash loans), an attacker could use frontrunning to gain an unfair advantage.

Proof of Concept:

Scenario 1: Malicious Owner Frontrunning Legitimate Submission

1. Owner1 submits a transaction:

   submitTransaction(0xRecipient, 10 ether)

2. The transaction's ID is deterministically set as transactions.length - 1 (e.g., txId = 5).

3. Malicious Owner2 monitors the mempool and submits a transaction with slightly higher gas:

   submitTransaction(0xAttacker, 10 ether)

4. If Owner2’s transaction gets mined first, it will take txId = 5, while Owner1’s original transaction is now txId = 6, altering execution expectations.

Recommended Mitigation:

Approach 1: Hash-Based Unique Transaction Identifiers

Instead of using the array index as an identifier, assign transactions a unique hash based on sender, recipient, and value.

Modification:

1. Introduce a mapping for transactions:

   mapping(bytes32 => Transaction) public transactions;

2. Modify submitTransaction to generate a unique transaction ID:

   function submitTransaction(address _to, uint256 _value) external onlyOwners {

   if (_to == address(0)) revert InvalidRecipient();

   if (_value == 0) revert InvalidAmount();

   ​

   bytes32 txId = keccak256(abi.encodePacked(msg.sender, _to, _value, block.timestamp));

   require(transactions[txId].to == address(0), "Transaction already exists");

   ​

   transactions[txId] = Transaction(_to, _value, false, false, false);

   emit TransactionCreated(txId, _to, _value);

   }

3. Update approveTransaction and executeTransaction to use txId:

   function approveTransaction(bytes32 _txId) external onlyOwners {

   Transaction storage txn = transactions[_txId];

   require(txn.to != address(0), "Invalid transaction ID");

   require(!txn.executed, "Transaction already executed");

   ​

   if (msg.sender == owner1) {

   if (txn.approvedByOwner1) revert AlreadyApproved();

   txn.approvedByOwner1 = true;

   } else {

   if (txn.approvedByOwner2) revert AlreadyApproved();

   txn.approvedByOwner2 = true;

   }

   ​

   emit TransactionApproved(_txId, msg.sender);

   }

Approach 2: Delayed Execution with Commitment Scheme

1. Submit a hashed transaction request first:

   bytes32 txHash = keccak256(abi.encodePacked(_to, _value, nonce));

2. Require the sender to later reveal the details in a second step.

3. Prevents mempool frontrunning since the full details are not visible.

Final Recommendation:

The hash-based unique transaction ID approach is the simplest and most efficient solution to mitigate frontrunning while maintaining contract usability. If stronger protection is needed, a commit-reveal scheme can be implemented for added security.