]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect: fix bug where rule without sid is accepted
authorJuliana Fajardini <jufajardini@gmail.com>
Thu, 17 Jun 2021 16:18:09 +0000 (17:18 +0100)
committerVictor Julien <victor@inliniac.net>
Thu, 1 Jul 2021 06:20:48 +0000 (08:20 +0200)
Before, if Suricata parsed a rule without a 'sid' option, instead of
failing that rule, the rule was parsed and attributed a sid 0.
Changes to:
detect-parse:
- add logic to filter out rules without sid;
- change unittest which didn't have a sid, but used to pass.
detect-sid: add unittest for rules without sid or with sid: 0

src/detect-parse.c
src/detect-sid.c

index 8c1bd2a87de2bbf7923f1f62eef9ee3660fc1bf5..13d54806167de5a1e77ced2696fa0be5816da8db 100644 (file)
@@ -1889,6 +1889,10 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s)
             SCReturnInt(0);
         }
     }
+    if (s->id == 0) {
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "Signature missing required value \"sid\".");
+        SCReturnInt(0);
+    }
     SCReturnInt(1);
 }
 
@@ -2807,13 +2811,15 @@ static int SigParseTest11(void)
 
     Signature *s = NULL;
 
-    s = DetectEngineAppendSig(de_ctx, "drop tcp any any -> any 80 (msg:\"Snort_Inline is blocking the http link\";) ");
+    s = DetectEngineAppendSig(de_ctx,
+            "drop tcp any any -> any 80 (msg:\"Snort_Inline is blocking the http link\"; sid:1;) ");
     if (s == NULL) {
         printf("sig 1 didn't parse: ");
         goto end;
     }
 
-    s = DetectEngineAppendSig(de_ctx, "drop tcp any any -> any 80 (msg:\"Snort_Inline is blocking the http link\"; sid:1;)            ");
+    s = DetectEngineAppendSig(de_ctx, "drop tcp any any -> any 80 (msg:\"Snort_Inline is blocking "
+                                      "the http link\"; sid:2;)            ");
     if (s == NULL) {
         printf("sig 2 didn't parse: ");
         goto end;
index 979a663ccfcef553d5d4d2c35df0f2604a64f101..d598908ecfe3b260846c2b8c5a0b33532c26c801 100644 (file)
@@ -119,6 +119,22 @@ static int SidTestParse03(void)
     PASS;
 }
 
+static int SidTestParse04(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF_NULL(de_ctx);
+
+    FAIL_IF_NOT_NULL(DetectEngineAppendSig(
+            de_ctx, "alert tcp any any -> any any (content:\"ABC\"; sid: 0;)"));
+
+    /* Let's also make sure that Suricata fails a rule which doesn't have a sid at all */
+    FAIL_IF_NOT_NULL(
+            DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (content:\"ABC\";)"));
+
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
+
 /**
  * \brief Register DetectSid unit tests.
  */
@@ -127,5 +143,6 @@ static void DetectSidRegisterTests(void)
     UtRegisterTest("SidTestParse01", SidTestParse01);
     UtRegisterTest("SidTestParse02", SidTestParse02);
     UtRegisterTest("SidTestParse03", SidTestParse03);
+    UtRegisterTest("SidTestParse04", SidTestParse04);
 }
 #endif /* UNITTESTS */
\ No newline at end of file