]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/isdataat: optimize recursion mismatches
authorVictor Julien <vjulien@oisf.net>
Mon, 25 Sep 2023 08:16:27 +0000 (10:16 +0200)
committerVictor Julien <victor@inliniac.net>
Thu, 7 Dec 2023 08:56:59 +0000 (09:56 +0100)
Since recursive content matching goes through the buffer from left to
right, it is possible to bail early when isdataat is part of the
recursive checking. If `isdataat:50,relative` fails for offset 10, it
will surely also fail for offset 20. So break inspection in such cases.

The exception is for dynamic isdataat, where the value is determined
by a prior byte_extract that may be updated during the recursion.

src/detect-engine-content-inspection.c
src/tests/detect-engine-content-inspection.c

index ac8a39226ab96e780e9410b36d4f885f9569e42c..76baead03513316ecf67e60815047cd2b3b9b81c 100644 (file)
@@ -408,23 +408,34 @@ static int DetectEngineContentInspectionInternal(DetectEngineThreadCtx *det_ctx,
                 SCLogDebug("det_ctx->buffer_offset + dataat %"PRIu32" > %"PRIu32, det_ctx->buffer_offset + dataat, buffer_len);
                 if (id->flags & ISDATAAT_NEGATED)
                     goto match;
+                if ((id->flags & ISDATAAT_OFFSET_VAR) == 0) {
+                    goto no_match_discontinue;
+                }
                 goto no_match;
             } else {
                 SCLogDebug("relative isdataat match");
-                if (id->flags & ISDATAAT_NEGATED)
+                if (id->flags & ISDATAAT_NEGATED) {
                     goto no_match;
+                }
                 goto match;
             }
         } else {
             if (dataat < buffer_len) {
                 SCLogDebug("absolute isdataat match");
-                if (id->flags & ISDATAAT_NEGATED)
+                if (id->flags & ISDATAAT_NEGATED) {
+                    if ((id->flags & ISDATAAT_OFFSET_VAR) == 0) {
+                        goto no_match_discontinue;
+                    }
                     goto no_match;
+                }
                 goto match;
             } else {
                 SCLogDebug("absolute isdataat mismatch, id->isdataat %"PRIu32", buffer_len %"PRIu32"", dataat, buffer_len);
                 if (id->flags & ISDATAAT_NEGATED)
                     goto match;
+                if ((id->flags & ISDATAAT_OFFSET_VAR) == 0) {
+                    goto no_match_discontinue;
+                }
                 goto no_match;
             }
         }
index 65a4f578b824bafe8f2c177b31cb3f4358c6b2e3..c4e1a3bdff68d8e62e35c2e24cdf02e6da4b65e0 100644 (file)
@@ -143,6 +143,10 @@ static int DetectEngineContentInspectionTest06(void) {
     // 6 steps: (1) a, (2) 1st b, (3) c not found, (4) 2nd b, (5) c found, isdataat
     TEST_RUN("ababc", 5, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:!1,relative;", true, 5);
     TEST_RUN("ababc", 5, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:1,relative;", false, 6);
+    TEST_RUN("abcabc", 6,
+            "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; "
+            "within:1; isdataat:10,relative;",
+            false, 4);
 
     TEST_RUN("ababcabc", 8, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:!1,relative;", true, 7);
     TEST_RUN("ababcabc", 8, "content:\"a\"; content:\"b\"; distance:0; within:1; content:\"c\"; distance:0; within:1; isdataat:1,relative;", true, 6);
@@ -228,6 +232,11 @@ static int DetectEngineContentInspectionTest10(void) {
     TEST_RUN("x9x9abcdefghi", 13, "content:\"x\"; byte_extract:1,0,data_size,string,relative; isdataat:data_size,relative;", true, 3);
     TEST_RUN("x9x9abcdefgh", 12, "content:\"x\"; byte_extract:1,0,data_size,string,relative; isdataat:!data_size,relative;", true, 5);
     TEST_RUN("x9x9abcdefgh", 12, "content:\"x\"; depth:1; byte_extract:1,0,data_size,string,relative; isdataat:!data_size,relative;", false, 3);
+    /* first isdataat should fail, second succeed */
+    TEST_RUN("x9x5abcdef", 10,
+            "content:\"x\"; byte_extract:1,0,data_size,string,relative; "
+            "isdataat:data_size,relative;",
+            true, 5);
     /* check for super high extracted values */
     TEST_RUN("100000000abcdefghi", 18, "byte_extract:0,0,data_size,string; isdataat:data_size;", false, 2);
     TEST_RUN("100000000abcdefghi", 18, "byte_extract:0,0,data_size,string; isdataat:!data_size;", true, 2);