Pull request #3227: http2_inspect: hardening

Merge in SNORT/snort3 from ~THOPETER/snort3:h2i23 to master

Squashed commit of the following:

commit 74e4038907b3f282fb03262caa3376caf19002e5
Author: Tom Peters <thopeter@cisco.com>
Date:   Tue Dec 21 14:21:22 2021 -0500

    http2_inspect: hardening

diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h
index 2fc6f244d99996c43041050cba6506122f4cf5e7..fbecab2a69ea86280b627529c090afa85765248d 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_flow_data.h
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.h
@@ -122,7 +122,7 @@ protected:
 
     // 0 element refers to client frame, 1 element refers to server frame
 
-    // There is currently one infraction and one event object per flow per direction.
+    // There are currently one infraction and one event object per flow per direction.
     Http2Infractions* const infractions[2] = { new Http2Infractions, new Http2Infractions };
     Http2EventGen* const events[2] = { new Http2EventGen, new Http2EventGen };
 
@@ -185,6 +185,7 @@ protected:
     uint32_t frame_header_offset[2] = { 0, 0 };
     uint32_t frame_data_offset[2] = { 0, 0 };
     uint32_t remaining_frame_octets[2] = { 0, 0 };
+    uint32_t running_total[2] = { 0, 0 };
     uint8_t remaining_padding_reassemble[2] = { 0, 0 };
     bool read_frame_header[2] = { false, false };
     bool continuation_frame[2] = { false, false };

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 5d3519f89003b871a0c46995e8bb769a7d19fd7c..88fb97107916d71d2d76e74b406b20aaba3fec4c 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -328,15 +328,18 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
     unsigned total, unsigned offset, const uint8_t* data, unsigned len, uint32_t flags,
     HttpCommon::SourceId source_id)
 {
-
     StreamBuffer frame_buf { nullptr, 0 };
 
-    if ( offset+len > total || total != session_data->bytes_scanned[source_id])
+    if ((session_data->running_total[source_id] != offset) ||
+        (total != session_data->bytes_scanned[source_id]) ||
+        (offset+len > total) ||
+        ((flags & PKT_PDU_TAIL) && (offset+len != total)))
     {
          assert(false);
          session_data->abort_flow[source_id] = true;
          return frame_buf;
     }
+    session_data->running_total[source_id] += len;
 
     if (session_data->frame_type[source_id] == FT_DATA)
     {
@@ -477,6 +480,7 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
             // but don't create pkt_data buffer
             frame_buf.data = (const uint8_t*)"";
         }
+        session_data->running_total[source_id] = 0;
         session_data->bytes_scanned[source_id] = 0;
     }