Pull request #3511: detection: fix the bug with qualified events

Merge in SNORT/snort3 from ~YVELYKOZ/snort3:detection_child_bug to master

Squashed commit of the following:

commit 5e7bd568b6dd21556bcb305f5f02366e374877ee
Author: Yehor Velykozhon <yvelykoz@cisco.com>
Date:   Wed Jul 6 18:52:38 2022 +0300

    detection: separate the branch/leaf result to different variables

    Evaluation function's return value is for branch results,
    while the flag in eval_data is for leaf results.

diff --git a/src/detection/detection_options.cc b/src/detection/detection_options.cc
index 75abf52e9b6c49bc93d0303ad5173ec81f5daa19..488bcb6a647cb3de40404302d5a802a26d140fde 100644 (file)
--- a/src/detection/detection_options.cc
+++ b/src/detection/detection_options.cc
@@ -479,6 +479,7 @@ int detection_option_node_evaluate(
                         fpAddMatch(p->context->otnx, otn);
                     }
                     result = rval = (int)IpsOption::MATCH;
+                    eval_data.leaf_reached = 1;
                 }
             }
             break;

diff --git a/src/detection/detection_options.h b/src/detection/detection_options.h
index aaf9f760b3388a351e6f6431a0db979a67e0d0d9..503de9c8c3e2007d30ad1ee521ae8909f3b856c2 100644 (file)
--- a/src/detection/detection_options.h
+++ b/src/detection/detection_options.h
@@ -110,8 +110,15 @@ struct detection_option_tree_root_t
 
 struct detection_option_eval_data_t
 {
+    detection_option_eval_data_t() = delete;
+
+    detection_option_eval_data_t(snort::Packet* p) :
+        pmd(nullptr), p(p), leaf_reached(0), flowbit_failed(0), flowbit_noalert(0)
+    { }
+
     void* pmd;
     snort::Packet* p;
+    char leaf_reached;
     char flowbit_failed;
     char flowbit_noalert;
 };

diff --git a/src/detection/fp_detect.cc b/src/detection/fp_detect.cc
index a18dd49c072d6034d1c22c8564a25e57d995e42b..03eb996184403b8875ab70142f521d312441d6f8 100644 (file)
--- a/src/detection/fp_detect.cc
+++ b/src/detection/fp_detect.cc
@@ -336,7 +336,7 @@ int fp_eval_option(void* v, Cursor& c, Packet* p)
     return opt->eval(c, p);
 }
 
-static int detection_option_tree_evaluate(detection_option_tree_root_t* root,
+static void detection_option_tree_evaluate(detection_option_tree_root_t* root,
     detection_option_eval_data_t& eval_data)
 {
     assert(root);
@@ -344,21 +344,17 @@ static int detection_option_tree_evaluate(detection_option_tree_root_t* root,
     RuleLatency::Context rule_latency_ctx(root, eval_data.p);
 
     if ( RuleLatency::suspended() )
-        return 0;
+        return;
 
     Cursor c(eval_data.p);
-    int rval = 0;
 
     debug_log(detection_trace, TRACE_RULE_EVAL, eval_data.p, "Starting tree eval\n");
 
     for ( int i = 0; i < root->num_children; ++i )
     {
-        // Increment number of events generated from that child
-        rval += detection_option_node_evaluate(root->children[i], eval_data, c);
+        detection_option_node_evaluate(root->children[i], eval_data, c);
     }
     clear_trace_cursor_info();
-
-    return rval;
 }
 
 static void rule_tree_match(
@@ -366,11 +362,8 @@ static void rule_tree_match(
 {
     PMX* pmx = (PMX*)user;
 
-    detection_option_eval_data_t eval_data;
-    eval_data.p = context->packet;
+    detection_option_eval_data_t eval_data(context->packet);
     eval_data.pmd = pmx->pmd;
-    eval_data.flowbit_failed = 0;
-    eval_data.flowbit_noalert = 0;
 
     print_pattern(pmx->pmd, eval_data.p);
 
@@ -399,9 +392,10 @@ static void rule_tree_match(
             return;
 
         detection_option_tree_root_t* root = (detection_option_tree_root_t*)tree;
-        int ret = detection_option_tree_evaluate(root, eval_data);
 
-        if ( ret )
+        detection_option_tree_evaluate(root, eval_data);
+
+        if ( eval_data.leaf_reached )
             pmqs.qualified_events++;
         else
             pmqs.non_qualified_events++;
@@ -1029,22 +1023,16 @@ static inline void eval_nfp(
             if ( fp->get_debug_print_nc_rules() )
                 LogMessage("NC-testing %u rules\n", port_group->nfp_rule_count);
 
-            detection_option_eval_data_t eval_data;
+            detection_option_eval_data_t eval_data(p);
 
-            eval_data.p = p;
-            eval_data.pmd = nullptr;
-            eval_data.flowbit_failed = 0;
-            eval_data.flowbit_noalert = 0;
+            debug_log(detection_trace, TRACE_RULE_EVAL, p,
+                "Testing non-content rules\n");
 
-            int rval = 0;
-            {
-                debug_log(detection_trace, TRACE_RULE_EVAL, p,
-                    "Testing non-content rules\n");
-                rval = detection_option_tree_evaluate(
-                    (detection_option_tree_root_t*)port_group->nfp_tree, eval_data);
-            }
+            detection_option_tree_root_t* root = (detection_option_tree_root_t*)port_group->nfp_tree;
+
+            detection_option_tree_evaluate(root, eval_data);
 
-            if (rval)
+            if (eval_data.leaf_reached)
                 pmqs.qualified_events++;
             else
                 pmqs.non_qualified_events++;