detect: fix tx iterator logic in detect

The 'tx_id' variable was used to be passed into the IterFunc as a
minumum tx to return. The IterFunc could then return either the tx
for that id, or a later one if that turned out to be the first available
tx.

The tx_id however, was still used for some things as if it was the
current tx id. Most importantly for setting the tx id for alert
ammending. So this could lead to alerts with missing or wrong
applayer records.

diff --git a/src/detect.c b/src/detect.c
index 30009ecc069aab5fff5b6b820c1f991a9e8fde85..0f9a1822fac8717ee1d5a5f5ca090c30d337696d 100644 (file)
--- a/src/detect.c
+++ b/src/detect.c
@@ -1359,7 +1359,7 @@ static void DetectRunTx(ThreadVars *tv,
     const AppProto alproto = f->alproto;
 
     const uint64_t total_txs = AppLayerParserGetTxCnt(f, alstate);
-    uint64_t tx_id = AppLayerParserGetTransactionInspectId(f->alparser, flow_flags);
+    uint64_t tx_id_min = AppLayerParserGetTransactionInspectId(f->alparser, flow_flags);
     const int tx_end_state = AppLayerParserGetStateProgressCompletionStatus(alproto, flow_flags);
 
     AppLayerGetTxIteratorFunc IterFunc = AppLayerGetTxIterator(ipproto, alproto);
@@ -1367,7 +1367,7 @@ static void DetectRunTx(ThreadVars *tv,
     memset(&state, 0, sizeof(state));
 
     while (1) {
-        AppLayerGetTxIterTuple ires = IterFunc(ipproto, alproto, alstate, tx_id, total_txs, &state);
+        AppLayerGetTxIterTuple ires = IterFunc(ipproto, alproto, alstate, tx_id_min, total_txs, &state);
         if (ires.tx_ptr == NULL)
             break;
 
@@ -1375,9 +1375,12 @@ static void DetectRunTx(ThreadVars *tv,
                 alstate, ires.tx_id, ires.tx_ptr, tx_end_state, flow_flags);
         if (tx.tx_ptr == NULL) {
             SCLogDebug("%p/%"PRIu64" no transaction to inspect",
-                    tx.tx_ptr, tx_id);
+                    tx.tx_ptr, tx_id_min);
+
+            tx_id_min++; // next (if any) run look for +1
             goto next;
         }
+        tx_id_min = tx.tx_id + 1; // next look for cur + 1
 
         uint32_t array_idx = 0;
         uint32_t total_rules = det_ctx->match_array_cnt;
@@ -1390,7 +1393,7 @@ static void DetectRunTx(ThreadVars *tv,
                     alstate, &tx);
             PACKET_PROFILING_DETECT_END(p, PROF_DETECT_PF_TX);
             SCLogDebug("%p/%"PRIu64" rules added from prefilter: %u candidates",
-                    tx.tx_ptr, tx_id, det_ctx->pmq.rule_id_array_cnt);
+                    tx.tx_ptr, tx.tx_id, det_ctx->pmq.rule_id_array_cnt);
 
             total_rules += det_ctx->pmq.rule_id_array_cnt;
             if (!(RuleMatchCandidateTxArrayHasSpace(det_ctx, total_rules))) {
@@ -1425,11 +1428,11 @@ static void DetectRunTx(ThreadVars *tv,
                 array_idx++;
 
                 SCLogDebug("%p/%"PRIu64" rule %u (%u) added from 'match' list",
-                        tx.tx_ptr, tx_id, s->id, id);
+                        tx.tx_ptr, tx.tx_id, s->id, id);
             }
         }
         SCLogDebug("%p/%"PRIu64" rules added from 'match' list: %u",
-                tx.tx_ptr, tx_id, array_idx - x); (void)x;
+                tx.tx_ptr, tx.tx_id, array_idx - x); (void)x;
 
         /* merge stored state into results */
         if (tx.de_state != NULL) {
@@ -1440,7 +1443,7 @@ static void DetectRunTx(ThreadVars *tv,
             const bool have_new_file = (tx.de_state->flags & DETECT_ENGINE_STATE_FLAG_FILE_NEW);
             if (have_new_file) {
                 SCLogDebug("%p/%"PRIu64" destate: need to consider new file",
-                        tx.tx_ptr, tx_id);
+                        tx.tx_ptr, tx.tx_id);
                 tx.de_state->flags &= ~DETECT_ENGINE_STATE_FLAG_FILE_NEW;
             }
 
@@ -1474,11 +1477,11 @@ static void DetectRunTx(ThreadVars *tv,
                         DetectRunTxSortHelper);
 
                 SCLogDebug("%p/%"PRIu64" rules added from 'continue' list: %u",
-                        tx.tx_ptr, tx_id, array_idx - old);
+                        tx.tx_ptr, tx.tx_id, array_idx - old);
             }
         }
 
-        det_ctx->tx_id = tx_id;
+        det_ctx->tx_id = tx.tx_id;
         det_ctx->tx_id_set = 1;
         det_ctx->p = p;
 
@@ -1497,37 +1500,37 @@ static void DetectRunTx(ThreadVars *tv,
                     if (det_ctx->tx_candidates[i].flags != NULL) {
                         i++;
                         SCLogDebug("%p/%"PRIu64" inspecting SKIP NEXT: sid %u (%u), flags %08x",
-                                tx.tx_ptr, tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
+                                tx.tx_ptr, tx.tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
                     } else if (det_ctx->tx_candidates[i+1].flags != NULL) {
                         SCLogDebug("%p/%"PRIu64" inspecting SKIP CURRENT: sid %u (%u), flags %08x",
-                                tx.tx_ptr, tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
+                                tx.tx_ptr, tx.tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
                         continue;
                     } else {
                         // if it's all the same, inspect the current one and skip next.
                         i++;
                         SCLogDebug("%p/%"PRIu64" inspecting SKIP NEXT: sid %u (%u), flags %08x",
-                                tx.tx_ptr, tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
+                                tx.tx_ptr, tx.tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
                     }
                 }
             }
 
             SCLogDebug("%p/%"PRIu64" inspecting: sid %u (%u), flags %08x",
-                    tx.tx_ptr, tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
+                    tx.tx_ptr, tx.tx_id, s->id, s->num, inspect_flags ? *inspect_flags : 0);
 
             if (inspect_flags) {
                 if (*inspect_flags & (DE_STATE_FLAG_FULL_INSPECT|DE_STATE_FLAG_SIG_CANT_MATCH)) {
                     SCLogDebug("%p/%"PRIu64" inspecting: sid %u (%u), flags %08x ALREADY COMPLETE",
-                            tx.tx_ptr, tx_id, s->id, s->num, *inspect_flags);
+                            tx.tx_ptr, tx.tx_id, s->id, s->num, *inspect_flags);
                     continue;
                 }
             }
 
             if (inspect_flags) {
                 /* continue previous inspection */
-                SCLogDebug("%p/%"PRIu64" Continueing sid %u", tx.tx_ptr, tx_id, s->id);
+                SCLogDebug("%p/%"PRIu64" Continueing sid %u", tx.tx_ptr, tx.tx_id, s->id);
             } else {
                 /* start new inspection */
-                SCLogDebug("%p/%"PRIu64" Start sid %u", tx.tx_ptr, tx_id, s->id);
+                SCLogDebug("%p/%"PRIu64" Start sid %u", tx.tx_ptr, tx.tx_id, s->id);
             }
 
             /* call individual rule inspection */
@@ -1542,9 +1545,9 @@ static void DetectRunTx(ThreadVars *tv,
                 if (s->action & ACTION_DROP)
                     alert_flags |= PACKET_ALERT_FLAG_DROP_FLOW;
 
-                SCLogDebug("%p/%"PRIu64" sig %u (%u) matched", tx.tx_ptr, tx_id, s->id, s->num);
+                SCLogDebug("%p/%"PRIu64" sig %u (%u) matched", tx.tx_ptr, tx.tx_id, s->id, s->num);
                 if (!(s->flags & SIG_FLAG_NOALERT)) {
-                    PacketAlertAppend(det_ctx, s, p, tx_id, alert_flags);
+                    PacketAlertAppend(det_ctx, s, p, tx.tx_id, alert_flags);
                 } else {
                     DetectSignatureApplyActions(p, s, alert_flags);
                 }
@@ -1564,7 +1567,7 @@ static void DetectRunTx(ThreadVars *tv,
         if (tx.tx_progress >= tx.tx_end_state) {
             new_detect_flags |= APP_LAYER_TX_INSPECTED_FLAG;
             SCLogDebug("%p/%"PRIu64" tx is done for direction %s. Flag %016"PRIx64,
-                    tx.tx_ptr, tx_id,
+                    tx.tx_ptr, tx.tx_id,
                     flow_flags & STREAM_TOSERVER ? "toserver" : "toclient",
                     new_detect_flags);
         }
@@ -1572,7 +1575,7 @@ static void DetectRunTx(ThreadVars *tv,
             new_detect_flags |= tx.prefilter_flags;
             SCLogDebug("%p/%"PRIu64" updated prefilter flags %016"PRIx64" "
                     "(was: %016"PRIx64") for direction %s. Flag %016"PRIx64,
-                    tx.tx_ptr, tx_id, tx.prefilter_flags, tx.prefilter_flags_orig,
+                    tx.tx_ptr, tx.tx_id, tx.prefilter_flags, tx.prefilter_flags_orig,
                     flow_flags & STREAM_TOSERVER ? "toserver" : "toclient",
                     new_detect_flags);
         }
@@ -1581,7 +1584,7 @@ static void DetectRunTx(ThreadVars *tv,
         {
             new_detect_flags |= tx.detect_flags;
             SCLogDebug("%p/%"PRIu64" Storing new flags %016"PRIx64" (was %016"PRIx64")",
-                    tx.tx_ptr, tx_id, new_detect_flags, tx.detect_flags);
+                    tx.tx_ptr, tx.tx_id, new_detect_flags, tx.detect_flags);
             AppLayerParserSetTxDetectFlags(ipproto, alproto, tx.tx_ptr,
                     flow_flags, new_detect_flags);
         }