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. 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 fe88d69e4aef..5a00c3332c77 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; } @@ -293,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) @@ -376,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); @@ -1001,6 +1032,59 @@ 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; +} + +/** + * \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; @@ -1071,8 +1155,10 @@ 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); UtRegisterTest("DetectByteMathContext01", DetectByteMathContext01); } #endif /* UNITTESTS */