Somnia Disclosed Report

https://github.com/hackenproof-public/somnia

Description: InFlightTransactionTracker present a memory leak which could be exploited by an attacker to degrade node performance (mainly memory pressure) and could potentialy OOM and crash the node.

When a transaction is sent by a user and reach a node RPC, it will first reach the node's mempool which use the InFlightTransactionTracker class to tracks transaction and predict if they can be executed or not with the current world state.

1. TryAddTransactionInternal gets called which will create an entry in the map for that sender. As the comment indicate, it will create a new entry if doesn't exist.

    // Get or create the in-flight account state.
    InFlightAccount& in_flight_account = in_flight_accounts[transaction.from];

2. Later once the last in-flight transaction is executed from this sender, it will be removed from the map (in_flight_accounts.erase).

  // One of the transactions which passed `TryAddTransaction` has been executed. Remove it from our
  // in-flight transactions.
  void TransactionExecuted(ExecutionThread, const TransactionView& transaction,
                           const TransactionReceipt& receipt) {
    // Get the account state, which must exist as there must be an in-flight account.
    InFlightAccount& in_flight_account = in_flight_accounts.at(transaction.from);

    if (!receipt.TransactionShouldBeRecorded()) {
      // This transaction was included, and was then not able to be recorded. We should not have
      // allowed a transaction through `TryAddTransaction` where this is the case, so the sender
      // must have added a transaction via another data chain.
      //
      // In this case, we block all new transactions until all in-flight transactions from this
      // sender have been executed, so that we can then make decisions based on the latest executed
      // state.
      in_flight_account.is_blocked_until_no_in_flight = true;
    }

    // Then remove this transaction as an in-flight transaction.
    RELEASE_ASSERT(in_flight_account.num_transactions_in_flight > 0);
    if (in_flight_account.num_transactions_in_flight == 1) {
      // This was the final in-flight transaction from this account. Just remove the account state.
      in_flight_accounts.erase(transaction.from);
      return;
    }
	...

This pattern present a memory leak in the following cases:

• ALL the transaction considered simulated (which mean received by a non-validator node) will NEVER be removed from the map as returning early, thus not incrementing num_transactions_in_flight, thus never reaching in_flight_accounts.erase code path.
• Any transaction that fails and end up resulting in CAN_NEVER_BE_EXECUTED will never be executed, thus never removed from the map.

This means a malicious user could send dummy transaction from different accounts (so sender change, which add a new entry in the map) which would fail validation (thus resulting in CAN_NEVER_BE_EXECUTED) and posting this againt all the nodes in the network to acheive greater damage. This attack would be FREE since no gas would be charged as not being executed at all since it happen at the mempool level so very early in the transaction stage.

Affected Code

MempoolStatusCode TryAddTransactionInternal(const WorldState& world_state, const TransactionView& transaction, bool simulate) {
    // Get or create the in-flight account state.
    InFlightAccount& in_flight_account = in_flight_accounts[transaction.from];

    if (in_flight_account.is_blocked_until_no_in_flight &&
        in_flight_account.num_transactions_in_flight > 0) {
      // This account has been blocked until there is no in-flight transactions. Don't let any more
      // transactions through.
      return MempoolStatusCode::HAS_IN_FLIGHT_TRANSACTIONS;
    }
	
	...
}


  void TransactionExecuted(ExecutionThread, const TransactionView& transaction, const TransactionReceipt& receipt) {
    // Get the account state, which must exist as there must be an in-flight account.
    InFlightAccount& in_flight_account = in_flight_accounts.at(transaction.from);

    if (!receipt.TransactionShouldBeRecorded()) {
      // This transaction was included, and was then not able to be recorded. We should not have
      // allowed a transaction through `TryAddTransaction` where this is the case, so the sender
      // must have added a transaction via another data chain.
      //
      // In this case, we block all new transactions until all in-flight transactions from this
      // sender have been executed, so that we can then make decisions based on the latest executed
      // state.
      in_flight_account.is_blocked_until_no_in_flight = true;
    }

    // Then remove this transaction as an in-flight transaction.
    RELEASE_ASSERT(in_flight_account.num_transactions_in_flight > 0);
    if (in_flight_account.num_transactions_in_flight == 1) {
      // This was the final in-flight transaction from this account. Just remove the account state.
      in_flight_accounts.erase(transaction.from);
      return;
    }
	...
	
}

Impact

• Memory Exhaustion: in_flight_accounts map grows without bound → OOM.
• Performance Degradation: Memory pressure due to memory leak.

Assets: somnia\mempool\in_flight_transaction_tracker.h

---

Classification

Impact Rate: 3/5

Likelihood Rate: 3/5

Severity: Medium

---

Recommendations

Remediation:

Ensure map grow is not unbounded as currently by removing entry when you know it will not be erased anyway.

• Remove right away entry for simulated case
• Apply the logic (^^) for all the returning case that will end up in CAN_NEVER_BE_EXECUTED

  MempoolStatusCode TryAddTransactionInternal(const WorldState& world_state, const TransactionView& transaction, bool simulate) {

    ...

    // If we assume all and only our in-flight transactions from this sender will be executed, this
    // transaction will be executable.
    if (simulate) {
+     if (in_flight_account.num_transactions_in_flight == 0) {
+	    // Remove if not needed anymore
+       in_flight_accounts.erase(transaction.from);
+     }	
	
      // We have been asked to only simulate the addition of this transaction, so stop here.
      return MempoolStatusCode::SUCCESS;
    }

---

Evidences

Reproduce:

The attack would happen as follow:

1. User send a transaction that explicitly fails validation in TryAddTransactionInternal, for instance ACCOUNT_DOES_NOT_EXIST code path, so send it from an unknown account.
2. A new entry is being added to in_flight_accounts which is never erased since never executed.