detect/bytemath: bound shift counts

doc/userguide/rules/payload-keywords.rst

@@ -569,6 +569,8 @@ 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.
+
 The result can be stored in a result variable and referenced by
 other rule options later in the rule.
 

rust/src/detect/byte_math.rs

@@ -348,11 +348,21 @@ fn parse_bytemath(input: &str) -> IResult<&str, DetectByteMathData, RuleParseErr
         )));
     }
 
-    // Using left/right shift further restricts the value of nbytes. Note that
-    // validation has already ensured nbytes is in [1..10]
+    // Using left/right shift further restricts the values of nbytes and rvalue.
+    // Note that validation has already ensured nbytes is in [1..10]
     match byte_math.oper {
-        ByteMathOperator::LeftShift | ByteMathOperator::RightShift if byte_math.nbytes > 4 => {
-            return Err(make_error(format!("nbytes must be 1 through 4 (inclusive) when used with \"<<\" or \">>\"; {} is not valid", byte_math.nbytes)));
+        ByteMathOperator::LeftShift | ByteMathOperator::RightShift => {
+            if byte_math.nbytes > 4 {
+                return Err(make_error(format!("nbytes must be 1 through 4 (inclusive) when used with \"<<\" or \">>\"; {} is not valid", byte_math.nbytes)));
+            }
+            // A shift of 64 or more always yields 0. Reject the literal form;
+            // the variable form is only known at match time.
+            if 0 == (byte_math.flags & DETECT_BYTEMATH_FLAG_RVALUE_VAR) && byte_math.rvalue >= 64 {
+                return Err(make_error(format!(
+                    "rvalue must be less than 64 when used with \"<<\" or \">>\"; {} is not valid",
+                    byte_math.rvalue
+                )));
+            }
         }
         _ => {}
     };
@@ -618,6 +628,21 @@ mod tests {
         );
     }
 
+    #[test]
+    // a literal rvalue of 64 or more is rejected with rshift/lshift; a variable
+    // rvalue is resolved at match time and cannot be checked here
+    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_err());
+        assert!(parse_bytemath("bytes 4, offset 3933, oper <<, rvalue 64, result foo").is_err());
+        assert!(parse_bytemath("bytes 4, offset 3933, oper >>, rvalue 100, result foo").is_err());
+        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(

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;
     }
 
@@ -999,6 +1003,52 @@ 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 is rejected at parse time.
+ */
+static int DetectByteMathParseTest17(void)
+{
+    DetectByteMathData *bmd = DetectByteMathParse(
+            NULL, "bytes 4, offset 2, oper >>, rvalue 64, result foo", NULL, NULL);
+    FAIL_IF_NOT_NULL(bmd);
+
+    bmd = DetectByteMathParse(
+            NULL, "bytes 4, offset 2, oper <<, rvalue 64, result foo", NULL, NULL);
+    FAIL_IF_NOT_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;
@@ -1069,8 +1119,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 */