From: Juliana Fajardini <jufajardini@oisf.net>
Date: Thu, 28 Jul 2022 23:05:47 +0000 (-0300)
Subject: detect/alert: ensure reject action is applied
X-Git-Tag: suricata-6.0.7~52
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=32f90371d90977c3b0ed1bc96c35c7bcee2e35ca;p=thirdparty%2Fsuricata.git

detect/alert: ensure reject action is applied

Bug 5458 states that the reject action is no longer working. While SV
tests that use the reject action still pass, it indeed seems that a
regression has happened with commit aa93984, because while the
function that applies rule actions to the flow (RuleActionToFlow) does
check for the reject action, the newly added function PacketApply
SignatureActions only checks for ACTION_DROP or ACTION_PASS when
deciding to call RuleActionToFlow.

Bug #5458

(cherry picked from commit 1f54e8611ab39ce3509280574d137df23c325658)
---

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index ec362932e0..bff6078aa5 100644
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -28,6 +28,7 @@
 #include "flow-private.h"
 
 #include "util-profiling.h"
+#include "util-validate.h"
 
 /** tag signature we use for tag alerts */
 static Signature g_tag_signature;
@@ -179,7 +180,9 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const uin
     SCLogDebug("packet %" PRIu64 " sid %u action %02x alert_flags %02x", p->pcap_cnt, s->id,
             s->action, alert_flags);
 
-    if (s->action & ACTION_DROP) {
+    /* REJECT also sets ACTION_DROP, just make it more visible with this check */
+    if (s->action & (ACTION_DROP | ACTION_REJECT_ANY)) {
+        /* PacketDrop will update the packet action, too */
         PacketDrop(p, s->action, PKT_DROP_REASON_RULES);
 
         if (p->alerts.drop.action == 0) {
@@ -190,6 +193,8 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const uin
         if ((p->flow != NULL) && (alert_flags & PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW)) {
             RuleActionToFlow(s->action, p->flow);
         }
+
+        DEBUG_VALIDATE_BUG_ON(!PacketTestAction(p, ACTION_DROP));
     } else {
         PACKET_UPDATE_ACTION(p, s->action);
 
@@ -402,4 +407,3 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
 
 }
 
-
diff --git a/src/detect-engine-threshold.c b/src/detect-engine-threshold.c
index 8400c93912..16e77b2b0d 100644
--- a/src/detect-engine-threshold.c
+++ b/src/detect-engine-threshold.c
@@ -300,11 +300,11 @@ static inline void RateFilterSetAction(Packet *p, PacketAlert *pa, uint8_t new_a
             pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED;
             break;
         case TH_ACTION_DROP:
-            PacketDrop(p, new_action, PKT_DROP_REASON_RULES_THRESHOLD);
+            PacketDrop(p, ACTION_DROP, PKT_DROP_REASON_RULES_THRESHOLD);
             pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED;
             break;
         case TH_ACTION_REJECT:
-            PACKET_REJECT(p);
+            PacketDrop(p, (ACTION_REJECT | ACTION_DROP), PKT_DROP_REASON_RULES_THRESHOLD);
             pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED;
             break;
         case TH_ACTION_PASS: