From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Thu, 3 Sep 2020 16:04:21 +0000 (+0000)
Subject: Merge pull request #2454 in SNORT/snort3 from ~THOPETER/snort3:nhttp149 to master
X-Git-Tag: 3.0.2-6~21
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9db679a153870918ad8081d6ef4595f1ca4ca923;p=thirdparty%2Fsnort3.git

Merge pull request #2454 in SNORT/snort3 from ~THOPETER/snort3:nhttp149 to master

Squashed commit of the following:

commit d0715acf006ad328ec79e7270eb02828d45ed0f6
Author: Tom Peters <thopeter@cisco.com>
Date:   Wed Sep 2 19:49:39 2020 -0400

    http_inspect: script detection and concurrency fixes
---

diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index 767557a62..c30854dff 100644
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -21,8 +21,9 @@
 #include "config.h"
 #endif
 
-#include "http_common.h"
 #include "http_cutter.h"
+
+#include "http_common.h"
 #include "http_enum.h"
 
 using namespace HttpEnums;
@@ -770,9 +771,8 @@ ScanResult HttpBodyH2Cutter::cut(const uint8_t* /*buffer*/, uint32_t length,
     }
 }
 
-// This method searches the input stream looking for the beginning of a script or other dangerous
-// content that requires accelerated blocking. Exactly what we are looking for is encapsulated in
-// dangerous().
+// This method searches the input stream looking for a script or other dangerous content that
+// requires accelerated blocking. Exactly what we are looking for is encapsulated in dangerous().
 //
 // Return value true indicates a match and enables the packet that completes the matching sequence
 // to be detained (detained inspection) or sent for partial inspection (script detection).
@@ -818,17 +818,21 @@ bool HttpBodyCutter::need_accelerated_blocking(const uint8_t* data, uint32_t len
     return false;
 }
 
-// Currently we do detained inspection when we see a javascript starting
+// Currently we do accelerated blocking when we see a javascript
 bool HttpBodyCutter::dangerous(const uint8_t* data, uint32_t length)
 {
     const uint8_t* input_buf = data;
     uint32_t input_length = length;
     uint8_t* decomp_output = nullptr;
 
-    // Zipped flows must be decompressed before we can check them. Unzipping for detained
-    // inspection is completely separate from the unzipping done later in reassemble().
+    // Zipped flows must be decompressed before we can check them. Unzipping for accelerated
+    // blocking is completely separate from the unzipping done later in reassemble().
     if ((compression == CMP_GZIP) || (compression == CMP_DEFLATE))
     {
+        // Previous decompression failures make it impossible to search for scripts
+        if (decompress_failed)
+            return true;
+
         const uint32_t decomp_buffer_size = MAX_OCTETS;
         decomp_output = new uint8_t[decomp_buffer_size];
 
@@ -843,6 +847,7 @@ bool HttpBodyCutter::dangerous(const uint8_t* data, uint32_t length)
         // work out we assume it could be dangerous.
         if (((ret_val != Z_OK) && (ret_val != Z_STREAM_END)) || (compress_stream->avail_in > 0))
         {
+            decompress_failed = true;
             delete[] decomp_output;
             return true;
         }
diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h
index ffc686a6c..f512f860d 100644
--- a/src/service_inspectors/http_inspect/http_cutter.h
+++ b/src/service_inspectors/http_inspect/http_cutter.h
@@ -117,6 +117,7 @@ private:
     bool detention_required = false;
     HttpEnums::CompressId compression;
     z_stream* compress_stream = nullptr;
+    bool decompress_failed = false;
     const uint8_t* match_string;
     const uint8_t* match_string_upper;
     uint8_t string_length;
diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc
index abc8876d0..3b2dc4ac3 100644
--- a/src/service_inspectors/http_inspect/http_flow_data.cc
+++ b/src/service_inspectors/http_inspect/http_flow_data.cc
@@ -53,12 +53,15 @@ const uint16_t HttpFlowData::memory_usage_estimate = sizeof(HttpFlowData) + size
 HttpFlowData::HttpFlowData() : FlowData(inspector_id)
 {
 #ifdef REG_TEST
-    seq_num = ++instance_count;
-    if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP) &&
-        !HttpTestManager::use_test_input(HttpTestManager::IN_HTTP))
+    if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP))
     {
-        fprintf(HttpTestManager::get_output_file(), "Flow Data construct %" PRIu64 "\n", seq_num);
-        fflush(HttpTestManager::get_output_file());
+        seq_num = ++instance_count;
+        if (!HttpTestManager::use_test_input(HttpTestManager::IN_HTTP))
+        {
+            fprintf(HttpTestManager::get_output_file(),
+                "Flow Data construct %" PRIu64 "\n", seq_num);
+            fflush(HttpTestManager::get_output_file());
+        }
     }
 #endif
     HttpModule::increment_peg_counts(PEG_CONCURRENT_SESSIONS);