detect/var: Restrict var usage to single buffer

doc/userguide/upgrade.rst

@@ -177,6 +177,15 @@ 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. Multi-buffer usage is not
+  guaranteed to work due to internal handling of variables. We recommend modifying
+  rules that are flagged via warning or launching Suricata with ``--strict-rule-keywords``.
+  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,25 @@ 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 *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,7 @@
 
 void DetectByteExtractRegister(void);
 
-SigMatch *DetectByteExtractRetrieveSMVar(const char *, int sm_list, const Signature *);
+const SigMatch *DetectByteExtractRetrieveSMVar(const char *, 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
@@ -27,30 +27,63 @@
 #include "detect-byte-extract.h"
 #include "detect-bytemath.h"
 
+// Once a variable is found, apply strict semantics (error) else warning
+static bool DetectByteListMismatch(bool any, int sm_list, int found_list, bool strict,
+        const char *arg, const DetectEngineCtx *de_ctx)
+{
+    if (any || sm_list == found_list)
+        return false;
+
+    if (strict) {
+        return true;
+    }
+
+    if (de_ctx) {
+        SCLogWarning("Using byte variable from a different buffer may produce indeterminate "
+                     "results; variable: \"%s\" at line %" PRId32 " from file %s; see issue #1412",
+                arg, de_ctx->rule_line, de_ctx->rule_file);
+    }
+    return false;
+}
+
 /**
  * \brief Used to retrieve args from BM.
  *
  * \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, const DetectEngineCtx *de_ctx)
 {
-    SigMatch *bed_sm = DetectByteExtractRetrieveSMVar(arg, sm_list, s);
+    bool any = sm_list == -1;
+    int found_list;
+    const SigMatch *bed_sm = DetectByteExtractRetrieveSMVar(arg, &found_list, s);
     if (bed_sm != NULL) {
+        if (DetectByteListMismatch(any, sm_list, found_list, strict, arg, de_ctx)) {
+            return false;
+        }
+
+        SCLogDebug("[buf] found %s; list wanted: sm_list: %d, list found: %d", arg, sm_list,
+                found_list);
         *index = ((SCDetectByteExtractData *)bed_sm->ctx)->local_id;
         return true;
     }
 
-    SigMatch *bmd_sm = DetectByteMathRetrieveSMVar(arg, sm_list, s);
+    const SigMatch *bmd_sm = DetectByteMathRetrieveSMVar(arg, &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, de_ctx)) {
+            *index = ((DetectByteMathData *)bmd_sm->ctx)->local_id;
+            SCLogDebug("[list] found %s; list wanted: sm_list: %d, list found: %d", arg, sm_list,
+                    found_list);
+            return true;
+        }
     }
+
     return false;
 }

src/detect-byte.h

@@ -27,6 +27,6 @@
 
 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 *, const DetectEngineCtx *);
 #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)) {
             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)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_jump - %s",
                     offset);

src/detect-bytemath.c

@@ -360,7 +360,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)) {
             SCLogError("unknown byte_ keyword var seen in byte_math - %s", nbytes);
             goto error;
         }
@@ -372,7 +373,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)) {
             SCLogError("unknown byte_ keyword var seen in byte_math - %s", rvalue);
             goto error;
         }
@@ -434,6 +436,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.
  *
@@ -442,32 +453,27 @@ 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 *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,7 @@
 
 void DetectBytemathRegister(void);
 
-SigMatch *DetectByteMathRetrieveSMVar(const char *, int sm_list, const Signature *);
+const SigMatch *DetectByteMathRetrieveSMVar(const char *, 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

@@ -231,8 +231,7 @@ int DetectBytetestDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
 
         SCLogDebug("comparing base %d string 0x%" PRIx64 " %s%u 0x%" PRIx64,
                data->base, val, (neg ? "!" : ""), data->op, data->value);
-    }
-    else {
+    } else {
         int endianness = (flags & DETECT_BYTETEST_LITTLE) ?
                           BYTE_LITTLE_ENDIAN : BYTE_BIG_ENDIAN;
         extbytes = ByteExtractUint64(&val, endianness, (uint16_t)nbytes, ptr);
@@ -259,6 +258,7 @@ int DetectBytetestDoMatch(DetectEngineThreadCtx *det_ctx, const Signature *s,
 
     /* Compare using the configured operator */
     match = 0;
+    SCLogDebug("val: %ld, value: %ld", val, value);
     switch (data->op) {
         case DETECT_BYTETEST_OP_EQ:
             if (val == value) {
@@ -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)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_test - %s",
                     value);
@@ -654,7 +655,8 @@ 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)) {
             SCLogError("Unknown byte_extract var "
                        "seen in byte_test - %s",
                     offset);
@@ -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)) {
             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)) {
             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)) {
             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)) {
             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)) {
             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)) {
             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, NULL)) {
         return luaL_error(L, "unknown byte_extract or byte_math variable: %s", name);
     }