From: Tom Peters (thopeter) <thopeter@cisco.com>
Date: Wed, 10 Nov 2021 17:59:11 +0000 (+0000)
Subject: Pull request #3127: BUG #704687: Hitting assert while processing partial trailer... 
X-Git-Tag: 3.1.17.0~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1e06c96e43b351fde3b35497717f44e192e74359;p=thirdparty%2Fsnort3.git

Pull request #3127: BUG #704687: Hitting assert while processing partial trailer truncated immediately after the frame header

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

Squashed commit of the following:

commit b5b4daddd2f0f0fcc5b7841aa27fca2b49a94aa1
Author: Maya Dagon <mdagon@cisco.com>
Date:   Wed Oct 20 16:46:41 2021 -0400

    http2_inspect: truncated trailers without frame data
---

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.h b/src/service_inspectors/http2_inspect/http2_headers_frame.h
index f71120de9..5235e6bc4 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.h
@@ -53,7 +53,6 @@ protected:
     Field http1_header;                 // finalized headers to be passed to http_inspect
     uint32_t xtradata_mask = 0;
     bool detection_required = false;
-    bool process_frame = true;
     Http2HpackDecoder* hpack_decoder;
     uint8_t hpack_headers_offset = 0;
 };
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
index e45bbdde1..03b7147eb 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
@@ -43,9 +43,6 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer,
     Http2HeadersFrameWithStartline(header_buffer, header_len, data_buffer, data_len, session_data_,
         source_id_, stream_)
 {
-    if (!process_frame)
-        return;
-
     if (source_id == SRC_CLIENT)
         start_line_generator = new Http2RequestLine(session_data->events[source_id],
             session_data->infractions[source_id]);
@@ -70,9 +67,6 @@ bool Http2HeadersFrameHeader::valid_sequence(Http2Enums::StreamState state)
 
 void Http2HeadersFrameHeader::analyze_http1()
 {
-    if (!process_frame)
-        return;
-
     HttpFlowData* http_flow;
     if (!process_start_line(http_flow, source_id))
         return;
diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
index 8638dad39..b6a625791 100644
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
@@ -45,9 +45,6 @@ Http2HeadersFrameTrailer::Http2HeadersFrameTrailer(const uint8_t* header_buffer,
     Http2HeadersFrame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_,
         stream_)
 {
-    if (!process_frame)
-        return;
-
     if (!(get_flags() & FLAG_END_STREAM))
     {
         // Trailers without END_STREAM flag set.
@@ -71,18 +68,16 @@ bool Http2HeadersFrameTrailer::valid_sequence(Http2Enums::StreamState state)
 
 void Http2HeadersFrameTrailer::analyze_http1()
 {
-    if (!process_frame)
-        return;
-
     HttpFlowData* const http_flow = stream->get_hi_flow_data();
     assert(http_flow);
 
+    const bool valid_headers = http1_header.length() > 0;
     if (http_flow->get_type_expected(source_id) != HttpEnums::SEC_TRAILER)
     {
         // http_inspect is not yet expecting trailers. Flush empty buffer through scan, reassemble,
         // and eval to prepare http_inspect for trailers.
         assert(http_flow->get_type_expected(source_id) == HttpEnums::SEC_BODY_H2);
-        stream->finish_msg_body(source_id, true, true); // calls http_inspect scan()
+        stream->finish_msg_body(source_id, valid_headers, true); // calls http_inspect scan()
 
         unsigned copied;
         const StreamBuffer stream_buf =
@@ -98,7 +93,7 @@ void Http2HeadersFrameTrailer::analyze_http1()
             dummy_pkt.dsize = stream_buf.length;
             dummy_pkt.data = stream_buf.data;
             session_data->hi->eval(&dummy_pkt);
-            assert (http_flow->get_type_expected(source_id) == HttpEnums::SEC_TRAILER);
+            assert (!valid_headers || http_flow->get_type_expected(source_id) == HttpEnums::SEC_TRAILER);
             if (http_flow->get_type_expected(source_id) == HttpEnums::SEC_ABORT)
             {
                 stream->set_state(source_id, STREAM_ERROR);
@@ -108,6 +103,12 @@ void Http2HeadersFrameTrailer::analyze_http1()
         }
     }
 
+    if (!valid_headers)
+    {
+        stream->set_state(source_id, STREAM_ERROR);
+        return;
+    }
+
     process_decoded_headers(http_flow, source_id);
 }