detect/var: Restrict var usage to single buffer

doc/userguide/upgrade.rst

@@ -176,6 +176,13 @@ Major changes
   content that matched just because it fell in the inspection chunk without wholly
   belonging to any one request/response may not match any longer.
 
+- If a byte keyword (such as ``byte_extract`` or ``byte_math``, etc) is used with
+  a variable, and that variable usage is with a buffer other than the one used
+  to create the variable, a warning is printed and the rule is loaded. The
+  command-line option ``--strict-rule-keywords`` should be used to produce
+  an error message and prevent the rule from loading. See
+  https://redmine.openinfosecfoundation.org/issues/1412 for more information.
+
 Removals
 ~~~~~~~~
 - The ssh keywords ``ssh.protoversion`` and ``ssh.softwareversion`` have been removed.

src/detect-byte-extract.c

@@ -26,6 +26,7 @@
 
 #include "detect.h"
 #include "detect-engine.h"
+#include "detect-byte.h"
 #include "detect-content.h"
 #include "detect-pcre.h"
 #include "detect-bytejump.h"
@@ -364,6 +365,15 @@ static void DetectByteExtractFree(DetectEngineCtx *de_ctx, void *ptr)
     SCByteExtractFree(ptr);
 }
 
+static inline bool DetectByteExtractSMNameMatch(const SigMatch *sm, const char *arg)
+{
+    if (sm->type == DETECT_BYTE_EXTRACT) {
+        const SCDetectByteExtractData *bed = (const SCDetectByteExtractData *)sm->ctx;
+        return strcmp(bed->name, arg) == 0;
+    }
+    return false;
+}
+
 /**
  * \brief Lookup the SigMatch for a named byte_extract variable.
  *
@@ -372,30 +382,26 @@ static void DetectByteExtractFree(DetectEngineCtx *de_ctx, void *ptr)
  *
  * \retval A pointer to the SigMatch if found, otherwise NULL.
  */
-SigMatch *DetectByteExtractRetrieveSMVar(const char *arg, int sm_list, const Signature *s)
+const SigMatch *DetectByteExtractRetrieveSMVar(
+        const char *arg, int sm_list, int *found_list, const Signature *s)
 {
     for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        SigMatch *sm = s->init_data->buffers[x].head;
+        const SigMatch *sm = s->init_data->buffers[x].head;
         while (sm != NULL) {
-            if (sm->type == DETECT_BYTE_EXTRACT) {
-                const SCDetectByteExtractData *bed = (const SCDetectByteExtractData *)sm->ctx;
-                if (strcmp(bed->name, arg) == 0) {
-                    return sm;
-                }
+            if (DetectByteExtractSMNameMatch(sm, arg)) {
+                *found_list = s->init_data->buffers[x].id;
+                return sm;
             }
             sm = sm->next;
         }
     }
 
     for (int list = 0; list < DETECT_SM_LIST_MAX; list++) {
-        SigMatch *sm = s->init_data->smlists[list];
+        const SigMatch *sm = s->init_data->smlists[list];
         while (sm != NULL) {
-            // Make sure that the linked buffers ore on the same list
-            if (sm->type == DETECT_BYTE_EXTRACT && (sm_list == -1 || sm_list == list)) {
-                const SCDetectByteExtractData *bed = (const SCDetectByteExtractData *)sm->ctx;
-                if (strcmp(bed->name, arg) == 0) {
-                    return sm;
-                }
+            if (DetectByteExtractSMNameMatch(sm, arg)) {
+                *found_list = list;
+                return sm;
             }
             sm = sm->next;
         }

src/detect-byte-extract.h

@@ -26,7 +26,8 @@
 
 void DetectByteExtractRegister(void);
 
-SigMatch *DetectByteExtractRetrieveSMVar(const char *, int sm_list, const Signature *);
+const SigMatch *DetectByteExtractRetrieveSMVar(
+        const char *, int sm_list, int *found_list, const Signature *);
 int DetectByteExtractDoMatch(DetectEngineThreadCtx *, const SigMatchData *, const Signature *,
         const uint8_t *, uint32_t, uint64_t *, uint8_t);
 

src/detect-byte.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2020 Open Information Security Foundation
+/* Copyright (C) 2020-2025 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -32,25 +32,35 @@
  *
  * \param arg The name of the variable being sought
  * \param s The signature to check for the variable
+ * \param strict Match if and only iff the list sought and the list found equal.
  * \param sm_list The caller's matching buffer
  * \param index When found, the value of the slot within the byte vars
  *
  * \retval true A match for the variable was found.
  * \retval false
  */
-bool DetectByteRetrieveSMVar(
-        const char *arg, const Signature *s, int sm_list, DetectByteIndexType *index)
+bool DetectByteRetrieveSMVar(const char *arg, const Signature *s, bool strict, int sm_list,
+        DetectByteIndexType *index, int rule_line)
 {
-    SigMatch *bed_sm = DetectByteExtractRetrieveSMVar(arg, sm_list, s);
+    bool any = sm_list == -1;
+    int found_list;
+    const SigMatch *bed_sm = DetectByteExtractRetrieveSMVar(arg, sm_list, &found_list, s);
     if (bed_sm != NULL) {
+        if (DetectByteListMismatch(any, sm_list, found_list, strict, arg, rule_line)) {
+            return false;
+        }
+
         *index = ((SCDetectByteExtractData *)bed_sm->ctx)->local_id;
         return true;
     }
 
-    SigMatch *bmd_sm = DetectByteMathRetrieveSMVar(arg, sm_list, s);
+    const SigMatch *bmd_sm = DetectByteMathRetrieveSMVar(arg, sm_list, &found_list, s);
     if (bmd_sm != NULL) {
-        *index = ((DetectByteMathData *)bmd_sm->ctx)->local_id;
-        return true;
+        if (!DetectByteListMismatch(any, sm_list, found_list, strict, arg, rule_line)) {
+            *index = ((DetectByteMathData *)bmd_sm->ctx)->local_id;
+            return true;
+        }
     }
+
     return false;
 }

src/detect-byte.h

@@ -27,6 +27,24 @@
 
 typedef uint8_t DetectByteIndexType;
 
-bool DetectByteRetrieveSMVar(const char *, const Signature *, int sm_list, DetectByteIndexType *);
+bool DetectByteRetrieveSMVar(const char *, const Signature *, bool strict, int sm_list,
+        DetectByteIndexType *, int rule_line);
+
+// Once a variable is found, apply strict semantics (error) else warning
+static inline bool DetectByteListMismatch(
+        bool any, int sm_list, int found_list, bool strict, const char *arg, int rule_line)
+{
+    if (any || sm_list == found_list)
+        return false;
+
+    if (strict) {
+        return true;
+    }
+
+    SCLogWarning("Using byte variable from a different buffer may produce indeterminate "
+                 "results; variable: \"%s\" at line %" PRId32 "",
+            arg, rule_line);
+    return false;
+}
 
 #endif /* SURICATA_DETECT_BYTE_H */

src/detect-bytejump.c

@@ -544,7 +544,8 @@ static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (nbytes != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(nbytes, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(nbytes, s, SigMatchStrictEnabled(DETECT_BYTEJUMP), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_jump - %s",
                     nbytes);
@@ -557,7 +558,8 @@ static int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (offset != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(offset, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(offset, s, SigMatchStrictEnabled(DETECT_BYTEJUMP), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_jump - %s",
                     offset);

src/detect-bytemath.c

@@ -358,7 +358,8 @@ static int DetectByteMathSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (nbytes != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(nbytes, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(nbytes, s, SigMatchStrictEnabled(DETECT_BYTEMATH), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("unknown byte_ keyword var seen in byte_math - %s", nbytes);
             goto error;
         }
@@ -370,7 +371,8 @@ static int DetectByteMathSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (rvalue != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(rvalue, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(rvalue, s, SigMatchStrictEnabled(DETECT_BYTEMATH), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("unknown byte_ keyword var seen in byte_math - %s", rvalue);
             goto error;
         }
@@ -432,6 +434,15 @@ static void DetectByteMathFree(DetectEngineCtx *de_ctx, void *ptr)
     SCByteMathFree(ptr);
 }
 
+static inline bool DetectByteMathSMNameMatch(const SigMatch *sm, const char *arg)
+{
+    if (sm->type == DETECT_BYTEMATH) {
+        const DetectByteMathData *bmd = (const DetectByteMathData *)sm->ctx;
+        return strcmp(bmd->result, arg) == 0;
+    }
+    return false;
+}
+
 /**
  * \brief Lookup the SigMatch for a named byte_math variable.
  *
@@ -440,32 +451,28 @@ static void DetectByteMathFree(DetectEngineCtx *de_ctx, void *ptr)
  *
  * \retval A pointer to the SigMatch if found, otherwise NULL.
  */
-SigMatch *DetectByteMathRetrieveSMVar(const char *arg, int sm_list, const Signature *s)
+const SigMatch *DetectByteMathRetrieveSMVar(
+        const char *arg, int sm_list, int *found_list, const Signature *s)
 {
     for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        SigMatch *sm = s->init_data->buffers[x].head;
+        const SigMatch *sm = s->init_data->buffers[x].head;
         while (sm != NULL) {
-            if (sm->type == DETECT_BYTEMATH) {
-                const DetectByteMathData *bmd = (const DetectByteMathData *)sm->ctx;
-                if (strcmp(bmd->result, arg) == 0) {
-                    SCLogDebug("Retrieved SM for \"%s\"", arg);
-                    return sm;
-                }
+            if (DetectByteMathSMNameMatch(sm, arg)) {
+                SCLogDebug("Retrieved SM for \"%s\"", arg);
+                *found_list = s->init_data->buffers[x].id;
+                return sm;
             }
             sm = sm->next;
         }
     }
 
     for (int list = 0; list < DETECT_SM_LIST_MAX; list++) {
-        SigMatch *sm = s->init_data->smlists[list];
+        const SigMatch *sm = s->init_data->smlists[list];
         while (sm != NULL) {
-            // Make sure that the linked buffers ore on the same list
-            if (sm->type == DETECT_BYTEMATH && (sm_list == -1 || sm_list == list)) {
-                const DetectByteMathData *bmd = (const DetectByteMathData *)sm->ctx;
-                if (strcmp(bmd->result, arg) == 0) {
-                    SCLogDebug("Retrieved SM for \"%s\"", arg);
-                    return sm;
-                }
+            if (DetectByteMathSMNameMatch(sm, arg)) {
+                SCLogDebug("Retrieved SM for \"%s\"", arg);
+                *found_list = list;
+                return sm;
             }
             sm = sm->next;
         }

src/detect-bytemath.h

@@ -26,7 +26,8 @@
 
 void DetectBytemathRegister(void);
 
-SigMatch *DetectByteMathRetrieveSMVar(const char *, int sm_list, const Signature *);
+const SigMatch *DetectByteMathRetrieveSMVar(
+        const char *, int sm_list, int *found_list, const Signature *);
 int DetectByteMathDoMatch(DetectEngineThreadCtx *, const DetectByteMathData *, const Signature *,
         const uint8_t *, const uint32_t, uint8_t, uint64_t, uint64_t *, uint8_t);
 

src/detect-bytetest.c

@@ -640,7 +640,8 @@ static int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (value != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(value, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(value, s, SigMatchStrictEnabled(DETECT_BYTETEST), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_test - %s",
                     value);
@@ -654,10 +655,11 @@ static int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (offset != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(offset, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(offset, s, SigMatchStrictEnabled(DETECT_BYTETEST), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_test - %s",
-                    offset);
+                    value);
             goto error;
         }
         data->offset = index;
@@ -668,7 +670,8 @@ static int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
     if (nbytes != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(nbytes, s, sm_list, &index)) {
+        if (!DetectByteRetrieveSMVar(nbytes, s, SigMatchStrictEnabled(DETECT_BYTETEST), sm_list,
+                    &index, de_ctx->rule_line)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_test - %s",
                     nbytes);

src/detect-depth.c

@@ -106,7 +106,8 @@ static int DetectDepthSetup (DetectEngineCtx *de_ctx, Signature *s, const char *
     }
     if (str[0] != '-' && isalpha((unsigned char)str[0])) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(str, s, -1, &index)) {
+        if (!DetectByteRetrieveSMVar(
+                    str, s, SigMatchStrictEnabled(DETECT_DEPTH), -1, &index, de_ctx->rule_line)) {
             SCLogError("unknown byte_ keyword var "
                        "seen in depth - %s.",
                     str);

src/detect-distance.c

@@ -108,7 +108,8 @@ static int DetectDistanceSetup (DetectEngineCtx *de_ctx, Signature *s,
     }
     if (str[0] != '-' && isalpha((unsigned char)str[0])) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(str, s, -1, &index)) {
+        if (!DetectByteRetrieveSMVar(str, s, SigMatchStrictEnabled(DETECT_DISTANCE), -1, &index,
+                    de_ctx->rule_line)) {
             SCLogError("unknown byte_ keyword var "
                        "seen in distance - %s",
                     str);

src/detect-isdataat.c

@@ -347,7 +347,8 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, const char *isda
 
     if (offset != NULL) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(offset, s, -1, &index)) {
+        if (!DetectByteRetrieveSMVar(offset, s, SigMatchStrictEnabled(DETECT_ISDATAAT), -1, &index,
+                    de_ctx->rule_line)) {
             SCLogError("Unknown byte_extract var "
                        "seen in isdataat - %s\n",
                     offset);

src/detect-offset.c

@@ -92,7 +92,8 @@ int DetectOffsetSetup (DetectEngineCtx *de_ctx, Signature *s, const char *offset
     }
     if (str[0] != '-' && isalpha((unsigned char)str[0])) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(str, s, -1, &index)) {
+        if (!DetectByteRetrieveSMVar(
+                    str, s, SigMatchStrictEnabled(DETECT_OFFSET), -1, &index, de_ctx->rule_line)) {
             SCLogError("unknown byte_ keyword var "
                        "seen in offset - %s.",
                     str);

src/detect-within.c

@@ -104,7 +104,8 @@ static int DetectWithinSetup(DetectEngineCtx *de_ctx, Signature *s, const char *
     }
     if (str[0] != '-' && isalpha((unsigned char)str[0])) {
         DetectByteIndexType index;
-        if (!DetectByteRetrieveSMVar(str, s, -1, &index)) {
+        if (!DetectByteRetrieveSMVar(
+                    str, s, SigMatchStrictEnabled(DETECT_WITHIN), -1, &index, de_ctx->rule_line)) {
             SCLogError("unknown byte_ keyword var "
                        "seen in within - %s",
                     str);

src/util-lua-bytevarlib.c

@@ -55,7 +55,7 @@ static int LuaBytevarMap(lua_State *L)
     }
 
     DetectByteIndexType idx;
-    if (!DetectByteRetrieveSMVar(name, s, -1, &idx)) {
+    if (!DetectByteRetrieveSMVar(name, s, false, -1, &idx, 1)) {
         luaL_error(L, "unknown byte_extract or byte_math variable: %s", name);
     }