From: Tom Peters (thopeter) <thopeter@cisco.com>
Date: Wed, 10 Nov 2021 19:11:10 +0000 (+0000)
Subject: Pull request #3139: BUG #705517 Http2HeadersFrame::clear is looking at server side... 
X-Git-Tag: 3.1.17.0~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1f8b481bf516077510a527941d148af1c278a4ed;p=thirdparty%2Fsnort3.git

Pull request #3139: BUG #705517 Http2HeadersFrame::clear is looking at server side stream state for push promise

Merge in SNORT/snort3 from ~MDAGON/snort3:push_promise2 to master

Squashed commit of the following:

commit f57c8f53f1fdfef5a73320471b8ef4369fba6f70
Author: Maya Dagon <mdagon@cisco.com>
Date:   Mon Oct 25 14:52:44 2021 -0400

    http2_inspect: push promise error state check
---

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.cc b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
index c451f7c0c..6ec1fd309 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
@@ -53,9 +53,14 @@ Http2HeadersFrame::Http2HeadersFrame(const uint8_t* header_buffer, const uint32_
     hpack_decoder = &session_data->hpack_decoder[source_id];
 }
 
+bool Http2HeadersFrame::in_error_state() const
+{
+    return stream->get_state(source_id) == STREAM_ERROR;
+}
+
 void Http2HeadersFrame::clear()
 {
-    if (session_data->abort_flow[source_id] || stream->get_state(source_id) == STREAM_ERROR)
+    if (session_data->abort_flow[source_id] || in_error_state())
         return;
     Packet dummy_pkt(false);
     dummy_pkt.flow = session_data->flow;
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.h b/src/service_inspectors/http2_inspect/http2_headers_frame.h
index 5235e6bc4..b40099209 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.h
@@ -49,6 +49,7 @@ protected:
     bool decode_headers(Http2StartLine* start_line_generator, bool trailers);
     void process_decoded_headers(HttpFlowData* http_flow, HttpCommon::SourceId hi_source_id);
     uint8_t get_flags_mask() const override;
+    virtual bool in_error_state() const;
 
     Field http1_header;                 // finalized headers to be passed to http_inspect
     uint32_t xtradata_mask = 0;
diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index e47fa964e..d77584d20 100644
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -204,6 +204,7 @@ void Http2Inspect::clear(Packet* p)
     session_data->stream_in_hi = NO_STREAM_ID;
     session_data->processing_stream_id = NO_STREAM_ID;
     session_data->processing_partial_header = false;
+    session_data->set_hi_msg_section(nullptr);
 }
 
 void Http2Inspect::show(const SnortConfig*) const
diff --git a/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc b/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
index a47578d54..91b90e10e 100644
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
@@ -150,6 +150,17 @@ uint32_t Http2PushPromiseFrame::get_promised_stream_id(Http2EventGen* const even
 uint8_t Http2PushPromiseFrame::get_flags_mask() const
 { return (FLAG_END_HEADERS|FLAG_PADDED); }
 
+bool Http2PushPromiseFrame::in_error_state() const
+{
+    // valid_sequence failures set error on source_id side.
+    // Header processing errors set error on client side.
+    // If client side was already in error state, valid_sequence
+    // would have failed.
+    return stream->get_state(SRC_CLIENT) == STREAM_ERROR ||
+        stream->get_state(source_id) == STREAM_ERROR;
+}
+
+
 #ifdef REG_TEST
 void Http2PushPromiseFrame::print_frame(FILE* output)
 {
diff --git a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
index 5cb58f933..d64f9392e 100644
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
@@ -59,6 +59,8 @@ private:
         HttpCommon::SourceId src_id, Http2Stream* stream);
     uint8_t get_flags_mask() const override;
 
+    bool in_error_state() const override;
+
     static const int32_t PROMISED_ID_LENGTH = 4;
 };
 #endif
diff --git a/src/service_inspectors/http_inspect/http_inspect.cc b/src/service_inspectors/http_inspect/http_inspect.cc
index c5218aee5..9d06c35d6 100755
--- a/src/service_inspectors/http_inspect/http_inspect.cc
+++ b/src/service_inspectors/http_inspect/http_inspect.cc
@@ -618,7 +618,7 @@ void HttpInspect::clear(Packet* p)
 
     if (session_data == nullptr)
     {
-        // assert(false); // FIXIT-M something wrong with H2I Push Promise triggers this.
+        assert(false);
         return;
     }