From 8beef976fd61edca9fdeae09e7047fcdc0a91327 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 17 Aug 2026 10:18:50 -0400 Subject: [PATCH 1/3] detect/bytemath: guard right shift against wide counts Zero the result when a byte_math right shift count reaches 64, the width of the uint64_t being shifted, so the operation no longer depends on behavior C11 6.5.7p3 leaves undefined. The left shift case has done this since 473ca6dcf4; the right shift case was left unguarded. DetectByteMathDoMatch() shifted by whatever count it was handed. On x86_64 the hardware masks the count to its low six bits, so a count of 64 became a shift of 0 and returned the extracted value unchanged instead of 0. That value is stored in det_ctx->byte_values[] and feeds any byte_test, isdataat, or content offset later in the signature, so the signature's verdict follows from an arithmetic result the standard does not define. The count reaches the shift from the wire. When byte_math names a variable for rvalue, DetectEngineContentInspectionInternal() reads it out of det_ctx->byte_values[] at detect-engine-content-inspection.c:614, where a preceding byte_extract stored bytes taken from the payload, so one payload byte of 0x40 sets the count to 64. Issue: 8845 Issue: 8902 (cherry picked from commit e5d035fd1658dbdfdab1ec3b6fb446f01c44d9bb) --- src/detect-bytemath.c | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/src/detect-bytemath.c b/src/detect-bytemath.c index fe88d69e4aef..a5bfd9e073d1 100644 --- a/src/detect-bytemath.c +++ b/src/detect-bytemath.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2020-2022 Open Information Security Foundation +/* Copyright (C) 2020-2026 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 @@ -189,7 +189,11 @@ int DetectByteMathDoMatch(DetectEngineThreadCtx *det_ctx, const DetectByteMathDa } break; case RightShift: - val >>= rvalue; + if (rvalue < 64) { + val >>= rvalue; + } else { + val = 0; + } break; } @@ -1001,6 +1005,31 @@ static int DetectByteMathPacket02(void) PASS; } +/** + * \test A payload-supplied shift count of 64 or more yields 0 instead of + * shifting a uint64_t by its own width. + */ +static int DetectByteMathPacket03(void) +{ + /* byte 0 is the shift count (64), byte 1 the value shifted, byte 2 the + * expected result */ + uint8_t buf[] = { 0x40, 0xff, 0x00 }; + + Packet *p = UTHBuildPacket(buf, sizeof(buf), IPPROTO_UDP); + FAIL_IF_NULL(p); + + /* 0xff >> 64 is 0 */ + FAIL_IF_NOT(UTHPacketMatchSig(p, "alert udp any any -> any any " + "(byte_extract: 1, 0, shift;" + "byte_math: bytes 1, offset 1, oper >>, rvalue shift, result " + "var;" + "byte_test: 1, =, var, 2;" + "sid:1;)")); + UTHFreePacket(p); + + PASS; +} + static int DetectByteMathContext01(void) { DetectEngineCtx *de_ctx = NULL; @@ -1073,6 +1102,7 @@ static void DetectByteMathRegisterTests(void) UtRegisterTest("DetectByteMathParseTest16", DetectByteMathParseTest16); UtRegisterTest("DetectByteMathPacket01", DetectByteMathPacket01); UtRegisterTest("DetectByteMathPacket02", DetectByteMathPacket02); + UtRegisterTest("DetectByteMathPacket03", DetectByteMathPacket03); UtRegisterTest("DetectByteMathContext01", DetectByteMathContext01); } #endif /* UNITTESTS */ From 2e43cbf66c49acc72444984bb4b93091c6a730d2 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 17 Aug 2026 10:19:34 -0400 Subject: [PATCH 2/3] detect/bytemath: warn on literal shift counts of 64 Warn from DetectByteMathSetup() when byte_math pairs << or >> with a literal rvalue of 64 or more, naming the signature by sid. Such a shift gives 0 for every packet: DetectByteMathDoMatch() zeroes the result once the count reaches 64, the width of the uint64_t being shifted. rvalue was bounded only to the u32 range, so the rule loaded and ran a shift whose result was 0 whatever the packet held. The rule still loads. main rejects it in 89d09c457c, the commit this one is derived from, but a released branch should not stop loading rules that loaded before. Passing byte_math to --strict-rule-keywords turns the warning into a load failure, the same escalation classtype and reference apply to values they cannot check. A variable rvalue draws no warning. It resolves to a preceding byte_extract or byte_math result, whose value is known only once the rule runs, so the guard in DetectByteMathDoMatch() stays the only check on that path. Issue: 8845 Issue: 8902 (cherry picked from commit 89d09c457ca66ac5ffdd33b22b1fec89a7d72178) --- rust/src/detect/byte_math.rs | 17 ++++++++++- src/detect-bytemath.c | 56 ++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/rust/src/detect/byte_math.rs b/rust/src/detect/byte_math.rs index c7447330fbec..58e44582391f 100644 --- a/rust/src/detect/byte_math.rs +++ b/rust/src/detect/byte_math.rs @@ -1,4 +1,4 @@ -/* Copyright (C) 2022 Open Information Security Foundation +/* Copyright (C) 2022-2026 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 @@ -617,6 +617,21 @@ mod tests { ); } + #[test] + // a shift count of 64 or more parses; the rule loads with a warning from + // DetectByteMathSetup() and the shift yields 0 at match time + fn test_parser_shift_rvalue() { + assert!(parse_bytemath("bytes 4, offset 3933, oper >>, rvalue 63, result foo").is_ok()); + assert!(parse_bytemath("bytes 4, offset 3933, oper <<, rvalue 63, result foo").is_ok()); + assert!(parse_bytemath("bytes 4, offset 3933, oper >>, rvalue 64, result foo").is_ok()); + assert!(parse_bytemath("bytes 4, offset 3933, oper <<, rvalue 64, result foo").is_ok()); + assert!(parse_bytemath("bytes 4, offset 3933, oper >>, rvalue 100, result foo").is_ok()); + assert!(parse_bytemath("bytes 4, offset 3933, oper +, rvalue 100, result foo").is_ok()); + assert!( + parse_bytemath("bytes 4, offset 3933, oper >>, rvalue myrvalue, result foo").is_ok() + ); + } + #[test] fn test_parser_bitmask_invalid() { assert!(parse_bytemath( diff --git a/src/detect-bytemath.c b/src/detect-bytemath.c index a5bfd9e073d1..5a00c3332c77 100644 --- a/src/detect-bytemath.c +++ b/src/detect-bytemath.c @@ -297,6 +297,31 @@ static int DetectByteMathSetup(DetectEngineCtx *de_ctx, Signature *s, const char if (data == NULL) goto error; + /* A shift of 64 or more clears the 64 bit value being shifted, so the + * result is 0 for every packet. Only a literal rvalue can be checked + * here; a variable one is read from the payload at match time. */ + if ((data->oper == LeftShift || data->oper == RightShift) && + !(data->flags & DETECT_BYTEMATH_FLAG_RVALUE_VAR) && data->rvalue >= 64) { + if (SigMatchStrictEnabled(DETECT_BYTEMATH)) { + SCLogError("byte_math rvalue %u is 64 or more, so \"%s\" always gives 0", data->rvalue, + data->oper == LeftShift ? "<<" : ">>"); + goto error; + } + if (s->id > 0) { + SCLogWarning("signature sid:%u: byte_math rvalue %u is 64 or more, so \"%s\" " + "always gives 0", + s->id, data->rvalue, data->oper == LeftShift ? "<<" : ">>"); + } else if (de_ctx != NULL && de_ctx->rule_file != NULL) { + SCLogWarning("signature at %s:%u: byte_math rvalue %u is 64 or more, so \"%s\" " + "always gives 0", + de_ctx->rule_file, de_ctx->rule_line, data->rvalue, + data->oper == LeftShift ? "<<" : ">>"); + } else { + SCLogWarning("byte_math rvalue %u is 64 or more, so \"%s\" always gives 0", + data->rvalue, data->oper == LeftShift ? "<<" : ">>"); + } + } + int sm_list; if (s->init_data->list != DETECT_SM_LIST_NOTSET) { if (DetectBufferGetActiveList(de_ctx, s) == -1) @@ -380,6 +405,8 @@ static int DetectByteMathSetup(DetectEngineCtx *de_ctx, Signature *s, const char SCLogError("unknown byte_ keyword var seen in byte_math - %s", rvalue); goto error; } + /* rvalue becomes a byte_values[] index here, so a check on the + * literal count has to run above this point. */ data->rvalue = index; data->flags |= DETECT_BYTEMATH_FLAG_RVALUE_VAR; SCFree(rvalue); @@ -1030,6 +1057,34 @@ static int DetectByteMathPacket03(void) PASS; } +/** + * \test A literal shift count of 64 or more parses and keeps its value, so + * DetectByteMathSetup() can warn about it. + */ +static int DetectByteMathParseTest17(void) +{ + DetectByteMathData *bmd = DetectByteMathParse( + NULL, "bytes 4, offset 2, oper >>, rvalue 64, result foo", NULL, NULL); + FAIL_IF_NULL(bmd); + FAIL_IF_NOT(bmd->oper == RightShift); + FAIL_IF_NOT(bmd->rvalue == 64); + DetectByteMathFree(NULL, bmd); + + bmd = DetectByteMathParse( + NULL, "bytes 4, offset 2, oper <<, rvalue 100, result foo", NULL, NULL); + FAIL_IF_NULL(bmd); + FAIL_IF_NOT(bmd->oper == LeftShift); + FAIL_IF_NOT(bmd->rvalue == 100); + DetectByteMathFree(NULL, bmd); + + bmd = DetectByteMathParse( + NULL, "bytes 4, offset 2, oper >>, rvalue 63, result foo", NULL, NULL); + FAIL_IF_NULL(bmd); + DetectByteMathFree(NULL, bmd); + + PASS; +} + static int DetectByteMathContext01(void) { DetectEngineCtx *de_ctx = NULL; @@ -1100,6 +1155,7 @@ static void DetectByteMathRegisterTests(void) UtRegisterTest("DetectByteMathParseTest14", DetectByteMathParseTest14); UtRegisterTest("DetectByteMathParseTest15", DetectByteMathParseTest15); UtRegisterTest("DetectByteMathParseTest16", DetectByteMathParseTest16); + UtRegisterTest("DetectByteMathParseTest17", DetectByteMathParseTest17); UtRegisterTest("DetectByteMathPacket01", DetectByteMathPacket01); UtRegisterTest("DetectByteMathPacket02", DetectByteMathPacket02); UtRegisterTest("DetectByteMathPacket03", DetectByteMathPacket03); From 395662a6fa785bb305028dc91c22dca40efbb9d3 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Mon, 17 Aug 2026 10:19:39 -0400 Subject: [PATCH 3/3] doc/bytemath: document the result of wide shifts Record beside the existing note about division by zero that << and >> give 0 when rvalue is 64 or more, that a rule giving such an rvalue as a number still loads with a warning, and that --strict-rule-keywords makes it fail to load instead. rvalue can name a byte_extract or byte_math variable rather than a number, and then the count is not known until the rule runs, so the rule text does not say whether it will reach 64. The note on main stops at the result, because 89d09c457c rejects such a rule there instead of warning about it. Issue: 8845 Issue: 8902 (cherry picked from commit 4a647784c77473aef5c84edc08b128427dd256bc) --- doc/userguide/rules/payload-keywords.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/userguide/rules/payload-keywords.rst b/doc/userguide/rules/payload-keywords.rst index 6daa0e14f12f..b591d749c041 100644 --- a/doc/userguide/rules/payload-keywords.rst +++ b/doc/userguide/rules/payload-keywords.rst @@ -535,6 +535,12 @@ When ``relative`` is included, there must be a previous ``content`` or ``pcre`` Note: if ``oper`` is ``/`` and the divisor is 0, there will never be a match on the ``byte_math`` keyword. +Note: if ``oper`` is ``<<`` or ``>>`` and ``rvalue`` is 64 or greater, the result is 0. +A rule that gives such an ``rvalue`` as a number still loads, with a warning, unless +``byte_math`` is passed to ``--strict-rule-keywords``, which makes the rule fail to +load instead. When ``rvalue`` names a variable the count is only known once the rule +runs. + The result can be stored in a result variable and referenced by other rule options later in the rule.