Merge pull request #2030 in SNORT/snort3 from ~THOPETER/snort3:nhttp135 to master

Squashed commit of the following:

commit d7b1e4a922555e1d5b046eaacb8f36849e56e1ac
Author: Tom Peters <thopeter@cisco.com>
Date:   Fri Feb 21 11:26:22 2020 -0500

    http_inspect: improve precautions for stream interactions

diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h
index e4fe48159c0ea65792e9a35a6ec472b9f45c3a21..13eb602c0b6814ba2ed72369de08ea47811dd24a 100644 (file)
--- a/src/service_inspectors/http_inspect/http_flow_data.h
+++ b/src/service_inspectors/http_inspect/http_flow_data.h
@@ -103,6 +103,7 @@ private:
     // *** StreamSplitter => Inspector (facts about the most recent message section)
     HttpEnums::SectionType section_type[2] = { HttpEnums::SEC__NOT_COMPUTE,
                                                 HttpEnums::SEC__NOT_COMPUTE };
+    int32_t octets_reassembled[2] = { HttpCommon::STAT_NOT_PRESENT, HttpCommon::STAT_NOT_PRESENT };
     int32_t num_head_lines[2] = { HttpCommon::STAT_NOT_PRESENT, HttpCommon::STAT_NOT_PRESENT };
     bool tcp_close[2] = { false, false };
     bool partial_flush[2] = { false, false };

diff --git a/src/service_inspectors/http_inspect/http_inspect.cc b/src/service_inspectors/http_inspect/http_inspect.cc
index f6f6f2cfad97ad4c6161c9e31ebbe1f028a67918..e2f43fd85aea3f3db4426ad427c5a13b4442b10a 100644 (file)
--- a/src/service_inspectors/http_inspect/http_inspect.cc
+++ b/src/service_inspectors/http_inspect/http_inspect.cc
@@ -359,9 +359,15 @@ void HttpInspect::eval(Packet* p)
 
     HttpFlowData* session_data = http_get_flow_data(p->flow);
 
-    // FIXIT-H Workaround for unexpected eval() calls
-    if (session_data->section_type[source_id] == SEC__NOT_COMPUTE)
+    // FIXIT-H Workaround for unexpected eval() calls. Convert to asserts when possible.
+    if ((session_data->section_type[source_id] == SEC__NOT_COMPUTE) ||
+        (session_data->type_expected[source_id] == SEC_ABORT)       ||
+        (session_data->octets_reassembled[source_id] != p->dsize))
+    {
+        session_data->type_expected[source_id] = SEC_ABORT;
         return;
+    }
+    session_data->octets_reassembled[source_id] = STAT_NOT_PRESENT;
 
     // Don't make pkt_data for headers available to detection
     if ((session_data->section_type[source_id] == SEC_HEADER) ||
@@ -415,7 +421,6 @@ bool HttpInspect::process(const uint8_t* data, const uint16_t dsize, Flow* const
 {
     HttpMsgSection* current_section;
     HttpFlowData* session_data = http_get_flow_data(flow);
-    assert(session_data != nullptr);
 
     if (!session_data->partial_flush[source_id])
         HttpModule::increment_peg_counts(PEG_INSPECT);

diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc
index c930070d95346e732e179b77a9e14d41335e3fcc..6f4bc4aacbc3f93b28e5a8b7cf255ec39a70eb33 100644 (file)
--- a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc
@@ -168,21 +168,13 @@ bool HttpStreamSplitter::init_partial_flush(Flow* flow)
 {
     Profile profile(HttpModule::get_profile_stats());
 
-    if (source_id != SRC_SERVER)
-    {
-        assert(false);
-        return false;
-    }
+    assert(source_id == SRC_SERVER);
 
     HttpFlowData* session_data = HttpInspect::http_get_flow_data(flow);
     assert(session_data != nullptr);
-    if ((session_data->type_expected[source_id] != SEC_BODY_CL)      &&
-        (session_data->type_expected[source_id] != SEC_BODY_OLD)     &&
-        (session_data->type_expected[source_id] != SEC_BODY_CHUNK))
-    {
-        assert(false);
-        return false;
-    }
+    assert((session_data->type_expected[source_id] == SEC_BODY_CL)      ||
+           (session_data->type_expected[source_id] == SEC_BODY_OLD)     ||
+           (session_data->type_expected[source_id] == SEC_BODY_CHUNK));
 
 #ifdef REG_TEST
     if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP) &&

diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
index fb979d7415a9eea5ed4d204c4d247e9315432d79..2ddc5927cd7a5c9a8ef89840d9656646a4afc7de 100644 (file)
--- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
@@ -276,6 +276,12 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total,
     }
 #endif
 
+    assert(session_data->type_expected[source_id] != SEC_ABORT);
+    if (session_data->section_type[source_id] == SEC__NOT_COMPUTE)
+    {
+        return { nullptr, 0 };
+    }
+
     // Sometimes it is necessary to reassemble zero bytes when a connection is closing to trigger
     // proper clean up. But even a zero-length buffer cannot be processed with a nullptr lest we
     // get in trouble with memcpy() (undefined behavior) or some library.
@@ -283,13 +289,6 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total,
     if (data == nullptr)
         data = (const uint8_t*)"";
 
-    // FIXIT-H Workaround for TP Bug 149662
-    if (session_data->section_type[source_id] == SEC__NOT_COMPUTE)
-    {
-        return { nullptr, 0 };
-    }
-
-    assert(session_data->section_type[source_id] != SEC__NOT_COMPUTE);
     uint8_t*& partial_buffer = session_data->partial_buffer[source_id];
     uint32_t& partial_buffer_length = session_data->partial_buffer_length[source_id];
     uint32_t& partial_raw_bytes = session_data->partial_raw_bytes[source_id];
@@ -424,6 +423,7 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total,
 
         http_buf.data = buffer;
         http_buf.length = buf_size;
+        session_data->octets_reassembled[source_id] = buf_size;
 
         buffer = nullptr;
         session_data->section_offset[source_id] = 0;