Merge pull request #2480 in SNORT/snort3 from ~KATHARVE/snort3:h2i_bug to master

Squashed commit of the following:

commit 84f09f6257a9f9af151b8526c94166c713fbb134
Author: Katura Harvey <katharve@cisco.com>
Date:   Tue Sep 8 12:03:09 2020 -0400

    http2_inspect: fix how implement_reassemble uses frame_type

diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
index 9b57ef38238410507ab43fda3f5dc716830fa181..df08d4c00566e0bc6123fa94443261721d313fe1 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -288,8 +288,12 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                 // We have the full frame header, compute some variables
                 const uint32_t frame_length = get_frame_length(session_data->
                     scan_frame_header[source_id]);
-                const uint8_t type = session_data->frame_type[source_id] = get_frame_type(
+                const uint8_t type = get_frame_type(
                     session_data->scan_frame_header[source_id]);
+                // Continuation frames are collapsed into the preceding Headers or Push Promise
+                // frame
+                if (type != FT_CONTINUATION)
+                    session_data->frame_type[source_id] = type;
                 const uint8_t frame_flags = get_frame_flags(session_data->
                     scan_frame_header[source_id]);
                 const uint32_t old_stream = session_data->current_stream[source_id];
@@ -477,6 +481,7 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
             // Copy headers
             if (session_data->use_leftover_hdr[source_id])
             {
+                assert(session_data->frame_header_offset[source_id] == 0);
                 memcpy(session_data->frame_header[source_id],
                     session_data->leftover_hdr[source_id], FRAME_HEADER_LENGTH);
                 session_data->frame_header_offset[source_id] += FRAME_HEADER_LENGTH;
@@ -503,23 +508,24 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
             session_data->remaining_frame_octets[source_id] =
                 get_frame_length(session_data->frame_header[source_id] +
                 session_data->frame_header_offset[source_id] - FRAME_HEADER_LENGTH);
-
             uint8_t frame_flags = get_frame_flags(session_data->frame_header[source_id] +
                 session_data->frame_header_offset[source_id] - FRAME_HEADER_LENGTH);
 
-            const uint8_t type = session_data->frame_type[source_id];
-            if ((type == FT_DATA) || (type == FT_HEADERS))
+            // Get the most recent frame header type
+            assert(session_data->frame_header_offset[source_id] >= FRAME_HEADER_LENGTH);
+            assert(session_data->frame_header_offset[source_id] % FRAME_HEADER_LENGTH == 0);
+            const uint8_t type = get_frame_type(session_data->frame_header[source_id] +
+                (session_data->frame_header_offset[source_id] - FRAME_HEADER_LENGTH));
+
+            // FIXIT-E Alert if padded flag is set but frame length is 0. Also alert if padded
+            // flag is set on an invalid frame type
+            if ((type == FT_HEADERS) and (frame_flags & PADDED) and
+                (get_frame_length(session_data->frame_header[source_id]) >= 1))
             {
-                if ((frame_flags & PADDED) &&
-                    (get_frame_length(session_data->frame_header[source_id]) >= 1))
-                {
-                    session_data->get_padding_len[source_id] = true;
-                }
+                session_data->get_padding_len[source_id] = true;
             }
         }
         while (data_offset < len);
-        session_data->frame_type[source_id] = get_frame_type(
-            session_data->frame_header[source_id]);
     }
 
     if (flags & PKT_PDU_TAIL)