Pull request #3483: http2_inspect: consider continuation when checking headers length

Merge in SNORT/snort3 from ~ADMAMOLE/snort3:scan_total to master

Squashed commit of the following:

commit 7e8952c3a39590fd7dff1d637b189ded8da70ce9
Author: Adrian Mamolea <admamole@cisco.com>
Date:   Wed Jun 22 11:27:53 2022 -0400

    http2_inspect: consider continuation when checking headers length

diff --git a/doc/reference/builtin_stubs.txt b/doc/reference/builtin_stubs.txt
index 7fca9a4da17c4ef11b4eb2964ced55b6a8c3bd7c..e269124b8da57da988b96a85b27f31fe88a4e9fc 100644 (file)
--- a/doc/reference/builtin_stubs.txt
+++ b/doc/reference/builtin_stubs.txt
@@ -1488,7 +1488,8 @@ Nonempty HTTP/2 Data frame where a message body was not expected.
 
 121:38
 
-HTTP/2 non-Data frame longer than 63780 bytes
+HTTP/2 non-Data frame longer than 63780 bytes. For HEADERS and PUSH_PROMISE frames this includes the
+size of any following continuation frames.
 
 121:39
 

diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h
index 0e79d357e881f583ae9586f3f339e07bae6d0f7e..5ffcedaed10fdddfdc5ac2b46a6774c53e5a2eca 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_flow_data.h
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.h
@@ -180,6 +180,7 @@ protected:
     bool abort_flow[2] = { false, false };
     bool processing_partial_header = false;
     std::queue<uint32_t> frame_lengths[2];
+    uint32_t accumulated_frame_length[2] = { 0, 0 };
 
     // Internal to reassemble()
     uint32_t frame_header_offset[2] = { 0, 0 };

diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
index 27f28dd67a1fa097d89ef8ea728fa7b84e2198ba..588e412c6e3228b1c93d4cc652d8ac6799780531 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
@@ -101,12 +101,18 @@ StreamSplitter::Status Http2StreamSplitter::scan(Packet* pkt, const uint8_t* dat
     if (session_data->abort_flow[source_id])
         return HttpStreamSplitter::status_value(StreamSplitter::ABORT, true);
 
-    const StreamSplitter::Status ret_val =
+    StreamSplitter::Status ret_val =
         implement_scan(session_data, data, length, flush_offset, source_id);
 
     session_data->bytes_scanned[source_id] += (ret_val == StreamSplitter::FLUSH)?
         *flush_offset : length;
 
+    if (ret_val == StreamSplitter::SEARCH && session_data->bytes_scanned[source_id] >= MAX_OCTETS)
+    {
+        assert(false);
+        ret_val = StreamSplitter::ABORT;
+    }
+
     if (ret_val == StreamSplitter::ABORT)
         session_data->abort_flow[source_id] = true;
 

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 d987953deee0fa699a4f6ad404411e28a82fbd9c..1896e61d59eed29d33c8cf464ff0206ed98e0afa 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -214,6 +214,10 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                 const uint8_t frame_flags = get_frame_flags(session_data->
                     scan_frame_header[source_id]);
 
+                uint32_t& accumulated_frame_length = session_data->accumulated_frame_length[source_id];
+                if ((type == FT_HEADERS || type == FT_PUSH_PROMISE) && !(frame_flags & FLAG_END_HEADERS))
+                    accumulated_frame_length = frame_length + FRAME_HEADER_LENGTH;
+
                 if (type != FT_CONTINUATION)
                 {
                     session_data->frame_type[source_id] = type;
@@ -236,9 +240,11 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                         *session_data->infractions[source_id] += INF_INVALID_FLAG;
                         session_data->events[source_id]->create_event(EVENT_INVALID_FLAG);
                     }
+                    accumulated_frame_length += frame_length + FRAME_HEADER_LENGTH;
                 }
 
-                if ((type != FT_DATA) && (frame_length + FRAME_HEADER_LENGTH > MAX_OCTETS))
+                if (((type == FT_CONTINUATION) && (accumulated_frame_length > MAX_OCTETS)) ||
+                    ((type != FT_DATA) && (frame_length + FRAME_HEADER_LENGTH > MAX_OCTETS)))
                 {
                     // FIXIT-E long non-data frames may need to be supported
                     *session_data->infractions[source_id] += INF_NON_DATA_FRAME_TOO_LONG;