detect/bytemath: reject literal shift counts of 64

Fail rule load when byte_math pairs << or >> with a literal rvalue of
64 or more. rvalue was bounded only to u32::MAX, so a rule shifting by
100 loaded and then produced 0 for every packet it inspected, spending
detection work on a comparison whose outcome was settled before the
first packet arrived.

A variable rvalue still loads. Its value comes from a byte_extract on
the packet and is not known until the rule runs, so the guard in
DetectByteMathDoMatch() stays the only check covering that path.

DetectByteMathParse() reports every SCByteMathParse() failure as
"invalid bytemath values", so the rejected rule is named by the
"error parsing signature" line that follows rather than by the reason
the parser gave.

Issue: 8845

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

@@ -1028,6 +1028,27 @@ static int DetectByteMathPacket03(void)
     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;
@@ -1098,6 +1119,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);