From 80abc22f6475b6a87a33166729a871203f34d578 Mon Sep 17 00:00:00 2001
From: Philippe Antoine <pantoine@oisf.net>
Date: Thu, 9 Nov 2023 16:15:36 +0100
Subject: [PATCH] http2: limit number of concurrent transactions

Ticket: 6481

Instead of just setting the old transactions to a drop state so
that they get later cleaned up by Suricata, fail creating new ones.

This is because one call to app-layer parsing can create many
transactions, and quadratic complexity could happen in one
single app-layer parsing because of find_or_create_tx
---
 rust/src/http2/detect.rs |  4 ++--
 rust/src/http2/http2.rs  | 47 +++++++++++++++++++++++++++-------------
 2 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs
index 53258b3aa7d8..52b41190555b 100644
--- a/rust/src/http2/detect.rs
+++ b/rust/src/http2/detect.rs
@@ -932,7 +932,7 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) {
         blocks,
     };
     let txdata = HTTP2FrameTypeData::HEADERS(hs);
-    let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer);
+    let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer).unwrap();
     tx.frames_ts.push(HTTP2Frame {
         header: head,
         data: txdata,
@@ -975,7 +975,7 @@ fn http2_tx_set_settings(state: &mut HTTP2State, input: &[u8]) {
             match parser::http2_parse_frame_settings(&dec) {
                 Ok((_, set)) => {
                     let txdata = HTTP2FrameTypeData::SETTINGS(set);
-                    let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer);
+                    let tx = state.find_or_create_tx(&head, &txdata, Direction::ToServer).unwrap();
                     tx.frames_ts.push(HTTP2Frame {
                         header: head,
                         data: txdata,
diff --git a/rust/src/http2/http2.rs b/rust/src/http2/http2.rs
index 047b41402e12..b62ccb985034 100644
--- a/rust/src/http2/http2.rs
+++ b/rust/src/http2/http2.rs
@@ -611,9 +611,21 @@ impl HTTP2State {
 
     pub fn find_or_create_tx(
         &mut self, header: &parser::HTTP2FrameHeader, data: &HTTP2FrameTypeData, dir: Direction,
-    ) -> &mut HTTP2Transaction {
+    ) -> Option<&mut HTTP2Transaction> {
         if header.stream_id == 0 {
-            return self.create_global_tx();
+            if self.transactions.len() >= unsafe { HTTP2_MAX_STREAMS } {
+                for tx_old in &mut self.transactions {
+                    if tx_old.state == HTTP2TransactionState::HTTP2StateTodrop {
+                        // loop was already run
+                        break;
+                    }
+                    tx_old.set_event(HTTP2Event::TooManyStreams);
+                    // use a distinct state, even if we do not log it
+                    tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
+                }
+                return None;
+            }
+            return Some(self.create_global_tx());
         }
         let sid = match data {
             //yes, the right stream_id for Suricata is not the header one
@@ -643,30 +655,31 @@ impl HTTP2State {
             let tx = &mut self.transactions[index - 1];
             tx.tx_data.update_file_flags(self.state_data.file_flags);
             tx.update_file_flags(tx.tx_data.file_flags);
-            return tx;
+            return Some(tx);
         } else {
-            let mut tx = HTTP2Transaction::new();
-            self.tx_id += 1;
-            tx.tx_id = self.tx_id;
-            tx.stream_id = sid;
-            tx.state = HTTP2TransactionState::HTTP2StateOpen;
             // do not use SETTINGS_MAX_CONCURRENT_STREAMS as it can grow too much
-            if self.transactions.len() > unsafe { HTTP2_MAX_STREAMS } {
-                // set at least one another transaction to the drop state
+            if self.transactions.len() >= unsafe { HTTP2_MAX_STREAMS } {
                 for tx_old in &mut self.transactions {
-                    if tx_old.state != HTTP2TransactionState::HTTP2StateTodrop {
-                        // use a distinct state, even if we do not log it
-                        tx_old.set_event(HTTP2Event::TooManyStreams);
-                        tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
+                    if tx_old.state == HTTP2TransactionState::HTTP2StateTodrop {
+                        // loop was already run
                         break;
                     }
+                    tx_old.set_event(HTTP2Event::TooManyStreams);
+                    // use a distinct state, even if we do not log it
+                    tx_old.state = HTTP2TransactionState::HTTP2StateTodrop;
                 }
+                return None;
             }
+            let mut tx = HTTP2Transaction::new();
+            self.tx_id += 1;
+            tx.tx_id = self.tx_id;
+            tx.stream_id = sid;
+            tx.state = HTTP2TransactionState::HTTP2StateOpen;
             tx.tx_data.update_file_flags(self.state_data.file_flags);
             tx.update_file_flags(tx.tx_data.file_flags);
             tx.tx_data.file_tx = STREAM_TOSERVER|STREAM_TOCLIENT; // might hold files in both directions
             self.transactions.push_back(tx);
-            return self.transactions.back_mut().unwrap();
+            return Some(self.transactions.back_mut().unwrap());
         }
     }
 
@@ -1038,6 +1051,10 @@ impl HTTP2State {
                     );
 
                     let tx = self.find_or_create_tx(&head, &txdata, dir);
+                    if tx.is_none() {
+                        return AppLayerResult::err();
+                    }
+                    let tx = tx.unwrap();
                     if reass_limit_reached {
                         tx.tx_data.set_event(HTTP2Event::ReassemblyLimitReached as u8);
                     }