From: Victor Julien <vjulien@oisf.net>
Date: Fri, 11 Apr 2025 04:45:40 +0000 (+0200)
Subject: detect: don't set conflicting packet/flow actions
X-Git-Tag: suricata-8.0.0-rc1~471
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1643b017b662ff8c5abc10847db016d50add6759;p=thirdparty%2Fsuricata.git

detect: don't set conflicting packet/flow actions

If for the same a packet a drop rule and a pass rule would match,
the applying of actions could be contradictionary:

- the drop would be applied to the packet
- the pass rule would also be considered, not overriding the drop,
  but still setting the flow pass flag.

This would lead to the packet being dropped, but the rest of the
flow getting passed, including retransmissions of the dropped
packet.

This patch only sets drop/pass actions if no conflicting action
has been set on the packet before. It respects the action-order.

Bug: #7653.
---

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index 1c940dfd07..0cb10012c3 100644
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -419,14 +419,30 @@ static inline void PacketAlertFinalizeProcessQueue(
                 }
             }
 
-            /* set actions on the flow */
-            FlowApplySignatureActions(p, pa, s, pa->flags);
+            bool skip_action_set = false;
+            if ((p->action & (ACTION_DROP | ACTION_ACCEPT)) != 0) {
+                if (p->action & ACTION_DROP) {
+                    if (pa->action & (ACTION_PASS | ACTION_ACCEPT)) {
+                        skip_action_set = true;
+                    }
+                } else {
+                    if (pa->action & (ACTION_DROP)) {
+                        skip_action_set = true;
+                    }
+                }
+            }
+            SCLogDebug("packet %" PRIu64 ": i:%u sid:%u skip_action_set %s", p->pcap_cnt, i, s->id,
+                    BOOL2STR(skip_action_set));
+            if (!skip_action_set) {
+                /* set actions on the flow */
+                FlowApplySignatureActions(p, pa, s, pa->flags);
 
-            SCLogDebug("det_ctx->alert_queue[i].action %02x (DROP %s, PASS %s)", pa->action,
-                    BOOL2STR(pa->action & ACTION_DROP), BOOL2STR(pa->action & ACTION_PASS));
+                SCLogDebug("det_ctx->alert_queue[i].action %02x (DROP %s, PASS %s)", pa->action,
+                        BOOL2STR(pa->action & ACTION_DROP), BOOL2STR(pa->action & ACTION_PASS));
 
-            /* set actions on packet */
-            PacketApplySignatureActions(p, s, pa);
+                /* set actions on packet */
+                PacketApplySignatureActions(p, s, pa);
+            }
         }
 
         /* skip firewall sigs following a drop: IDS mode still shows alerts after an alert. */