From: Victor Julien <vjulien@oisf.net>
Date: Mon, 25 Apr 2022 16:00:24 +0000 (+0200)
Subject: detect: fix rule inspection order
X-Git-Tag: suricata-7.0.0-beta1~694
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F7308%2Fhead;p=thirdparty%2Fsuricata.git

detect: fix rule inspection order

Fix rules from the 'match' list getting added to the tx candidates list
unsorted. In some cases this could lead to the same sid getting inspected
twice leading to a DEBUG_VALIDATION_BUG_ON trigger.

Bug: #5144.
---

diff --git a/src/detect.c b/src/detect.c
index 6a0d034ba9..7ff2638dc3 100644
--- a/src/detect.c
+++ b/src/detect.c
@@ -1320,6 +1320,7 @@ static void DetectRunTx(ThreadVars *tv,
         }
         tx_id_min = tx.tx_id + 1; // next look for cur + 1
 
+        bool do_sort = false; // do we need to sort the tx candidate list?
         uint32_t array_idx = 0;
         uint32_t total_rules = det_ctx->match_array_cnt;
         total_rules += (tx.de_state ? tx.de_state->cnt : 0);
@@ -1370,8 +1371,9 @@ static void DetectRunTx(ThreadVars *tv,
                         tx.tx_ptr, tx.tx_id, s->id, id);
             }
         }
-        SCLogDebug("%p/%"PRIu64" rules added from 'match' list: %u",
-                tx.tx_ptr, tx.tx_id, array_idx - x); (void)x;
+        do_sort = (array_idx > x); // sort if match added anything
+        SCLogDebug("%p/%" PRIu64 " rules added from 'match' list: %u", tx.tx_ptr, tx.tx_id,
+                array_idx - x);
 
         /* merge stored state into results */
         if (tx.de_state != NULL) {
@@ -1411,14 +1413,15 @@ static void DetectRunTx(ThreadVars *tv,
                     array_idx++;
                 }
             }
-            if (old && old != array_idx) {
-                qsort(det_ctx->tx_candidates, array_idx, sizeof(RuleMatchCandidateTx),
-                        DetectRunTxSortHelper);
-
-                SCLogDebug("%p/%"PRIu64" rules added from 'continue' list: %u",
-                        tx.tx_ptr, tx.tx_id, array_idx - old);
-            }
+            do_sort |= (old && old != array_idx); // sort if continue list adds sids
+            SCLogDebug("%p/%" PRIu64 " rules added from 'continue' list: %u", tx.tx_ptr, tx.tx_id,
+                    array_idx - old);
         }
+        if (do_sort) {
+            qsort(det_ctx->tx_candidates, array_idx, sizeof(RuleMatchCandidateTx),
+                    DetectRunTxSortHelper);
+        }
+
 #ifdef PROFILING
         if (array_idx >= de_ctx->profile_match_logging_threshold)
             RulesDumpTxMatchArray(det_ctx, scratch->sgh, p, tx.tx_id, array_idx, x);