Merge pull request #2641 in SNORT/snort3 from ~KATHARVE/snort3:h2i_ss_fix to master

Squashed commit of the following:

commit 59e4058b0b81e8c526ace95e04589dbcae6632ab
Author: Katura Harvey <katharve@cisco.com>
Date:   Tue Nov 24 11:05:24 2020 -0500

    http2_inspect: fix empty queue access and some bookkeeping

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
index 31a378159b4b015b9d9f7714ca6ceb86d7b140a2..ae54b8a7f9c77ef58039cce30861532bc79a6a7c 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
@@ -38,14 +38,15 @@ Http2DataCutter::Http2DataCutter(Http2FlowData* _session_data, HttpCommon::Sourc
 { }
 
 StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t length,
-    uint32_t* flush_offset, uint32_t& data_offset, uint32_t frame_len, uint8_t frame_flags)
+    uint32_t* flush_offset, uint32_t& data_offset, uint8_t frame_flags)
 {
     const uint32_t cur_data_offset = data_offset;
     if (frame_bytes_seen == 0)
     {
-        data_len = frame_len;
+        assert(session_data->frame_lengths[source_id].size() == 1);
+        data_len = session_data->frame_lengths[source_id].front() -
+            session_data->padding_length[source_id];
         data_bytes_read = 0;
-        frame_bytes_seen = FRAME_HEADER_LENGTH;
 
         if (frame_flags & PADDED)
         {
@@ -63,7 +64,6 @@ StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t lengt
     frame_bytes_seen += cur_pos - data_offset;
     data_offset = cur_pos;
     *flush_offset = cur_pos;
-    session_data->scan_remaining_frame_octets[source_id] = frame_len - frame_bytes_seen;
 
     session_data->stream_in_hi = session_data->current_stream[source_id];
 
@@ -85,10 +85,12 @@ StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t lengt
             data_bytes_read -= unused_input;
             data_offset -= unused_input;
             *flush_offset -= unused_input;
+            session_data->scan_remaining_frame_octets[source_id] -= http_flush_offset;
         }
         else if (scan_result == StreamSplitter::SEARCH)
         {
             bytes_sent_http += cur_data;
+            session_data->scan_remaining_frame_octets[source_id] -= cur_data;
         }
         else
             assert(false);
@@ -98,7 +100,6 @@ StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t lengt
     {
         // Done with this frame, cleanup
         session_data->header_octets_seen[source_id] = 0;
-        session_data->scan_remaining_frame_octets[source_id] = 0;
         session_data->scan_state[source_id] = SCAN_FRAME_HEADER;
         frame_bytes_seen = 0;
 

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.h b/src/service_inspectors/http2_inspect/http2_data_cutter.h
index e1f1d512303e66b163b2cfe2e8d598dda37c3e6d..e692117e58159f668c195ddddf56b5b2944a0d2d 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.h
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.h
@@ -32,8 +32,7 @@ class Http2DataCutter
 public:
     Http2DataCutter(Http2FlowData* flow_data, HttpCommon::SourceId src_id);
     snort::StreamSplitter::Status scan(const uint8_t* data, uint32_t length,
-        uint32_t* flush_offset, uint32_t& data_offset, uint32_t frame_len,
-        uint8_t frame_flags);
+        uint32_t* flush_offset, uint32_t& data_offset, uint8_t frame_flags);
     void reassemble(const uint8_t* data, unsigned len);
 
 private:

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 e8bcd53138dab21253a608f99a28095310ce6d79..c90d45ef2eac8a70caa49f806f8d5c621ac1d4da 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -197,6 +197,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                         session_data->remaining_data_padding[source_id] <= (length - data_offset) ?
                         session_data->remaining_data_padding[source_id] : (length - data_offset);
                     session_data->remaining_data_padding[source_id] -= avail;
+                    session_data->scan_remaining_frame_octets[source_id] -= avail;
                     session_data->payload_discard[source_id] = true;
                     *flush_offset = avail;
                     return StreamSplitter::FLUSH;
@@ -297,6 +298,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                         session_data->padding_length[source_id];
                 }
                 session_data->scan_remaining_frame_octets[source_id] -= 1;
+                assert(!session_data->frame_lengths[source_id].empty());
                 if (session_data->padding_length[source_id] >
                     session_data->frame_lengths[source_id].back() - 1)
                 {
@@ -317,9 +319,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
             case SCAN_DATA:
             case SCAN_EMPTY_DATA:
             {
-                const uint32_t frame_length = session_data->frame_lengths[source_id].back();
-                const uint8_t type = get_frame_type(
-                    session_data->scan_frame_header[source_id]);
+                const uint8_t type = get_frame_type(session_data->scan_frame_header[source_id]);
                 const uint8_t frame_flags = get_frame_flags(session_data->
                     scan_frame_header[source_id]);
                 if (session_data->frame_type[source_id] != FT_DATA)
@@ -334,8 +334,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                     if (stream && stream->is_open(source_id))
                     {
                         status = session_data->data_cutter[source_id].scan(
-                            data, length, flush_offset, data_offset,
-                            frame_length - session_data->padding_length[source_id], frame_flags);
+                            data, length, flush_offset, data_offset, frame_flags);
                     }
                     else
                     {