diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index c6697a7576c8..cf0e33665807 100644 --- a/doc/userguide/upgrade.rst +++ b/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. diff --git a/src/detect-byte-extract.c b/src/detect-byte-extract.c index 3b691373261e..534823e6904f 100644 --- a/src/detect-byte-extract.c +++ b/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; } diff --git a/src/detect-byte-extract.h b/src/detect-byte-extract.h index 2214e81bbb87..54c156accc3e 100644 --- a/src/detect-byte-extract.h +++ b/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); diff --git a/src/detect-byte.c b/src/detect-byte.c index 3f8735b11979..49ae739c6c57 100644 --- a/src/detect-byte.c +++ b/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; } diff --git a/src/detect-byte.h b/src/detect-byte.h index fd251885120f..2b13f96cb517 100644 --- a/src/detect-byte.h +++ b/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 */ diff --git a/src/detect-bytejump.c b/src/detect-bytejump.c index 6b0363f0985f..af574ca97445 100644 --- a/src/detect-bytejump.c +++ b/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); diff --git a/src/detect-bytemath.c b/src/detect-bytemath.c index 2be6af4af463..b1633ce709b5 100644 --- a/src/detect-bytemath.c +++ b/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; } diff --git a/src/detect-bytemath.h b/src/detect-bytemath.h index 85ac9077eed9..e5846c79cfa9 100644 --- a/src/detect-bytemath.h +++ b/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); diff --git a/src/detect-bytetest.c b/src/detect-bytetest.c index e73b9bdd1b60..4bf4c655ed68 100644 --- a/src/detect-bytetest.c +++ b/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); diff --git a/src/detect-depth.c b/src/detect-depth.c index 7c0bf5bfe5f3..ba5f34c9d68d 100644 --- a/src/detect-depth.c +++ b/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); diff --git a/src/detect-distance.c b/src/detect-distance.c index 43b48ba799cc..63aa7def98d3 100644 --- a/src/detect-distance.c +++ b/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); diff --git a/src/detect-isdataat.c b/src/detect-isdataat.c index 7709568159b7..1c45985fced6 100644 --- a/src/detect-isdataat.c +++ b/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); diff --git a/src/detect-offset.c b/src/detect-offset.c index 91d02b2882e3..b6128417930c 100644 --- a/src/detect-offset.c +++ b/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); diff --git a/src/detect-within.c b/src/detect-within.c index f18a9347db8d..73808b140f75 100644 --- a/src/detect-within.c +++ b/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); diff --git a/src/util-lua-bytevarlib.c b/src/util-lua-bytevarlib.c index bea9a4ce9481..5c50e88d56ff 100644 --- a/src/util-lua-bytevarlib.c +++ b/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); }