From: Tom Peters (thopeter) <thopeter@cisco.com>
Date: Fri, 1 Jul 2022 20:40:36 +0000 (+0000)
Subject: Pull request #3483: http2_inspect: consider continuation when checking headers length
X-Git-Tag: 3.1.34.0~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7f6af85d25497923432b55f059072042a75ec2af;p=thirdparty%2Fsnort3.git

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 7fca9a4da..e269124b8 100644
--- 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 0e79d357e..5ffcedaed 100644
--- 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 27f28dd67..588e412c6 100644
--- 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 d987953de..1896e61d5 100644
--- 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;