Pull request #3127: BUG #704687: Hitting assert while processing partial trailer...

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 f71120de9c86559fc07225e79acd7f304edb010e..5235e6bc474e078ce0df75d3c646502b2d2a3c63 100644 (file)
--- 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 e45bbdde19d59b8516a416550cda017454862149..03b7147eb41cad34c6d89aab3d1c5f34bf3739c0 100644 (file)
--- 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 8638dad3906b0b5ba4598103ce1d9c79329beee0..b6a625791bb273b8177798512013caf387d33c08 100644 (file)
--- 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);
 }