Merge pull request #1802 in SNORT/snort3 from ~DERAMADA/snort3:h2i_header_decode_erro...

Squashed commit of the following:

commit e68cab344dfd15d2c1abbfb214409c6c22d0c741
Author: deramada <deramada@cisco.com>
Date:   Tue Oct 15 15:40:55 2019 -0400

    http2_inspect: abort on header decode error

diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h
index deab023e062dfb12bf38dbbf974c2f9e9d460d01..4401cb1427530873d01ddd7a318614779f46287d 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_enum.h
+++ b/src/service_inspectors/http2_inspect/http2_enum.h
@@ -31,8 +31,9 @@ static const int FRAME_HEADER_LENGTH = 9;
 static const uint32_t HTTP2_GID = 121;
 
 // Frame type codes (fourth octet of frame header)
-enum FrameType { FT_DATA=0, FT_HEADERS=1, FT_PRIORITY=2, FT_RST_STREAM=3, FT_SETTINGS=4,
-    FT_PUSH_PROMISE=5, FT_PING=6, FT_GOAWAY=7, FT_WINDOW_UPDATE=8, FT_CONTINUATION=9, FT__NONE=255 };
+enum FrameType : uint8_t { FT_DATA=0, FT_HEADERS=1, FT_PRIORITY=2, FT_RST_STREAM=3, FT_SETTINGS=4,
+    FT_PUSH_PROMISE=5, FT_PING=6, FT_GOAWAY=7, FT_WINDOW_UPDATE=8, FT_CONTINUATION=9, FT__ABORT=254,
+    FT__NONE=255 };
 
 // Message buffers available to clients
 // This enum must remain synchronized with Http2Api::classic_buffer_names[]

diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h
index 8e920f171c56924f262dfea8a2bbe2d38631f692..503ec856db8206b824b4c908e4d0517e893cb2b5 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_flow_data.h
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.h
@@ -84,6 +84,9 @@ protected:
     bool header_coming[2]  = { false, false };
     bool payload_discard[2] = { false, false };
     uint32_t frames_aggregated[2] = { 0, 0 };
+
+    // Used by scan, reassemble and eval to communicate
+    uint8_t frame_type[2] = { Http2Enums::FT__NONE, Http2Enums::FT__NONE };
     
     // Internal to reassemble()
     Http2Hpack hpack[2];

diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index a555d4b9a23e3dbd141cbb8d5e7921727da8eb53..5508afd6b6c81536db66c31929b5db8bb8a3220b 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -98,6 +98,13 @@ void Http2Inspect::eval(Packet* p)
     Http2FlowData* const session_data =
         (Http2FlowData*)p->flow->get_flow_data(Http2FlowData::inspector_id);
 
+    // FIXIT-H Workaround for unexpected eval() calls
+    // Avoid eval if scan/reassemble aborts
+    if (session_data->frame_type[source_id] == FT__NONE)
+        return;
+
+    session_data->frame_type[source_id] = FT__NONE;
+    
     set_file_data(session_data->frame_data[source_id],
         session_data->frame_data_size[source_id]);
     session_data->frame_in_detection = true;

diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
index faf8f0990f10bb974d349e488f5f08447d192987..60fb7bd631988935c85837ad3413a68c9f1546b4 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter.cc
@@ -54,6 +54,13 @@ StreamSplitter::Status Http2StreamSplitter::scan(Packet* pkt, const uint8_t* dat
         Http2Module::increment_peg_counts(PEG_FLOW);
     }
 
+    // General mechanism to abort using scan
+    if (session_data->frame_type[source_id] == FT__ABORT)
+    {
+        session_data->frame_type[source_id] = FT__NONE;
+        return HttpStreamSplitter::status_value(StreamSplitter::ABORT, true);
+    }
+
 #ifdef REG_TEST
     uint32_t dummy_flush_offset;
 

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 9d81778c6cd0105fc91d4c19f1c14db7cb142c92..481d47c095f23e2c770012919fd4024b31961837 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -402,6 +402,8 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to
                     // Since this doesn't go to detection, clear() doesn't get called, so need to
                     // clear frame data from flow data directly
                     session_data->clear_frame_data(source_id);
+
+                    session_data->frame_type[source_id] = FT__ABORT;
                     return frame_buf;
                 }
             }
@@ -410,6 +412,7 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to
         // create pkt_data buffer
         frame_buf.data = (const uint8_t*)"";
     }
+    session_data->frame_type[source_id] = get_frame_type(session_data->frame_header[source_id]);
     return frame_buf;
 }