Pull request #3129: Handling of PDUs disorder for inline/external JavaScript normaliz...

Merge in SNORT/snort3 from ~SVLASIUK/snort3:js_pdu_disorder to master

Squashed commit of the following:

commit 529713b1874e9c23516290dae9b3ed80a80276c9
Author: Serhii Vlasiuk <svlasiuk@cisco.com>
Date:   Fri Oct 22 17:06:50 2021 +0300

    http_inspect: handle PDUs disorder for inline/external JavaScript normalization

diff --git a/doc/reference/builtin_stubs.txt b/doc/reference/builtin_stubs.txt
index 950be585320c7de9405c0ac26330ba95ce91085d..8cb4ac9a05bb56c2b80f6a09902b3c8f3e08b168 100644 (file)
--- a/doc/reference/builtin_stubs.txt
+++ b/doc/reference/builtin_stubs.txt
@@ -1214,6 +1214,15 @@ resources.
 
 Consecutive commas in HTTP Accept-Encoding header
 
+119:273
+
+This alert is raised for the following situation. During JavaScript normalization middle
+PDUs can be missed and not normalized. Usually it happens when rules have file_data and
+js_data ips options and fast-pattern (FP) search is applying to file_data. Some PDUs don’t
+match file_data FP search and JavaScript normalization won't be executed for these PDUs.
+The normalization of the following PDUs for inline/external scripts will be stopped for
+current request within the flow.
+
 121:1
 
 invalid flag set on HTTP/2 frame

diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index d1c0a42ab21dcb04edd770c68e9839743fd6f645..9dfdf66eb1861ef770d3073293ffe5feee1fd228 100755 (executable)
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -282,6 +282,7 @@ enum Infraction
     INF_CHUNK_OVER_MAXIMUM = 128,
     INF_LONG_HOST_VALUE = 129,
     INF_ACCEPT_ENCODING_CONSECUTIVE_COMMAS = 130,
+    INF_JS_PDU_MISS = 131,
     INF__MAX_VALUE
 };
 
@@ -413,6 +414,7 @@ enum EventSid
     EVENT_JS_IDENTIFIER_OVERFLOW = 270,
     EVENT_JS_SCOPE_NEST_OVFLOW = 271,
     EVENT_ACCEPT_ENCODING_CONSECUTIVE_COMMAS = 272,
+    EVENT_JS_PDU_MISS = 273,
     EVENT__MAX_VALUE
 };
 

diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc
index 84d8680210f36f80563b4cadbd4fdcf9a1852fea..059fbbc6aa5f490496489c34da4c927839762aae 100644 (file)
--- a/src/service_inspectors/http_inspect/http_flow_data.cc
+++ b/src/service_inspectors/http_inspect/http_flow_data.cc
@@ -242,6 +242,12 @@ void HttpFlowData::garbage_collect()
 }
 
 #ifndef UNIT_TEST_BUILD
+void HttpFlowData::reset_js_pdu_idx()
+{
+    js_pdu_idx = pdu_idx = 0;
+    js_data_lost_once = false;
+}
+
 void HttpFlowData::reset_js_ident_ctx()
 {
     if (js_ident_ctx)
@@ -279,6 +285,13 @@ snort::JSNormalizer& HttpFlowData::acquire_js_ctx(int32_t ident_depth, size_t no
     return *js_normalizer;
 }
 
+bool HttpFlowData::is_pdu_missed()
+{
+    bool pdu_missed = ((pdu_idx - js_pdu_idx) > 1);
+    js_pdu_idx = pdu_idx;
+    return pdu_missed;
+}
+
 void HttpFlowData::release_js_ctx()
 {
     js_continue = false;

diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h
index 0f22644396dfa880f3f86eec7d4c07fef80aaf8c..87f7976a23f7f5c87a5ff08a255d3a1e1dbe7b69 100644 (file)
--- a/src/service_inspectors/http_inspect/http_flow_data.h
+++ b/src/service_inspectors/http_inspect/http_flow_data.h
@@ -193,6 +193,9 @@ private:
     HttpTransaction** pipeline = nullptr;
     int16_t pipeline_front = 0;
     int16_t pipeline_back = 0;
+    uint32_t pdu_idx = 0;
+    uint32_t js_pdu_idx = 0;
+    bool js_data_lost_once = false;
     bool pipeline_overflow = false;
     bool pipeline_underflow = false;
 
@@ -214,11 +217,13 @@ private:
     bool js_continue = false;
     bool js_built_in_event = false;
 
+    void reset_js_pdu_idx();
     void reset_js_ident_ctx();
     snort::JSNormalizer& acquire_js_ctx(int32_t ident_depth, size_t norm_depth,
         uint8_t max_template_nesting, uint32_t max_scope_depth,
         const std::unordered_set<std::string>& built_in_ident);
     void release_js_ctx();
+    bool is_pdu_missed();
 
     bool cutover_on_clear = false;
     bool ssl_search_abandoned = false;

diff --git a/src/service_inspectors/http_inspect/http_msg_body.cc b/src/service_inspectors/http_inspect/http_msg_body.cc
index d342026957078c18d267b9dc27c40d65d96ed2a5..0d3205ce156d669fb5d6c18fd8560bfa57aafa1c 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_body.cc
+++ b/src/service_inspectors/http_inspect/http_msg_body.cc
@@ -181,6 +181,8 @@ void HttpMsgBody::analyze()
             else
                 do_legacy_js_normalization(decompressed_file_body, js_norm_body);
 
+            ++session_data->pdu_idx;
+
             const int32_t detect_length =
                 (js_norm_body.length() <= session_data->detect_depth_remaining[source_id]) ?
                 js_norm_body.length() : session_data->detect_depth_remaining[source_id];
@@ -335,10 +337,22 @@ void HttpMsgBody::fd_event_callback(void* context, int event)
 
 void HttpMsgBody::do_enhanced_js_normalization(const Field& input, Field& output)
 {
+    if (session_data->js_data_lost_once)
+        return;
+
+    auto infractions = transaction->get_infractions(source_id);
     auto back = !session_data->partial_flush[source_id];
     auto http_header = get_header(source_id);
     auto normalizer = params->js_norm_param.js_norm;
-    auto infractions = transaction->get_infractions(source_id);
+
+    if (session_data->is_pdu_missed())
+    {
+        *infractions += INF_JS_PDU_MISS;
+        session_data->events[HttpCommon::SRC_SERVER]->create_event(EVENT_JS_PDU_MISS);
+
+        session_data->js_data_lost_once = true;
+        return;
+    }
 
     if (http_header and http_header->is_external_js())
         normalizer->do_external(input, output, infractions, session_data, back);

diff --git a/src/service_inspectors/http_inspect/http_msg_request.cc b/src/service_inspectors/http_inspect/http_msg_request.cc
index 3d4587a09f5eedd482025955465d7c72d621e3d7..bc1a57d4cc15ec2c75f40142b04768770cbe251e 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_request.cc
+++ b/src/service_inspectors/http_inspect/http_msg_request.cc
@@ -42,6 +42,7 @@ HttpMsgRequest::HttpMsgRequest(const uint8_t* buffer, const uint16_t buf_size,
     get_related_sections();
     session_data->release_js_ctx();
     session_data->reset_js_ident_ctx();
+    session_data->reset_js_pdu_idx();
 }
 
 HttpMsgRequest::~HttpMsgRequest()

diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc
index 689ab31dabc839467620f3e1570f77c0de3bb130..8de2047b4be59b5d2bddb0aa8b71c1127d666ff6 100755 (executable)
--- a/src/service_inspectors/http_inspect/http_tables.cc
+++ b/src/service_inspectors/http_inspect/http_tables.cc
@@ -333,6 +333,7 @@ const RuleMap HttpModule::http_events[] =
     { EVENT_JS_SCOPE_NEST_OVFLOW,       "JavaScript scope nesting is over capacity" },
     { EVENT_ACCEPT_ENCODING_CONSECUTIVE_COMMAS, "Consecutive commas in HTTP Accept-Encoding "
                                         "header" },
+    { EVENT_JS_PDU_MISS,                "missed PDUs during JavaScript normalization" },
     { 0, nullptr }
 };