detect: Validate that NOOPT options don't have optvals

Without this, a simple typo between : and ; is able to hide actual bugs
in rules.

I discovered 2 bugs in ET open ruleset this way.

(cherry picked from commit 10ea60a237cf41ddd10f7a887e2824b4b8e1c419)

diff --git a/src/detect-parse.c b/src/detect-parse.c
index e87e640df8301a6a554a190b19b015698abeb893..85b4e7464b384e1eec0b5ac8ea6e8942f5fe694a 100644 (file)
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -704,8 +704,14 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr,
 
     if (!(st->flags & (SIGMATCH_NOOPT|SIGMATCH_OPTIONAL_OPT))) {
         if (optvalue == NULL || strlen(optvalue) == 0) {
-            SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid formatting or malformed option to %s keyword: \'%s\'",
-                    optname, optstr);
+            SCLogError(SC_ERR_INVALID_SIGNATURE,
+                    "invalid formatting or malformed option to %s keyword: '%s'", optname, optstr);
+            goto error;
+        }
+    } else if (st->flags & SIGMATCH_NOOPT) {
+        if (optvalue && strlen(optvalue)) {
+            SCLogError(SC_ERR_INVALID_SIGNATURE, "unexpected option to %s keyword: '%s'", optname,
+                    optstr);
             goto error;
         }
     }

diff --git a/src/tests/detect-parse.c b/src/tests/detect-parse.c
index b7fb452b386bce4320ca3762b3610b4fe165d1d3..d807c838f8c38596033378e90d15bbf3ee00a747 100644 (file)
--- a/src/tests/detect-parse.c
+++ b/src/tests/detect-parse.c
@@ -38,6 +38,23 @@ static int DetectParseTest01 (void)
     PASS;
 }
 
+/**
+ * \test DetectParseTestNoOpt  is a regression test to make sure that we reject
+ * any signature where a NOOPT rule option is given a value. This can hide rule
+ * errors which make other options disappear, eg: foo: bar: baz; where "foo" is
+ * the NOOPT option, we will end up with a signature which is missing "bar".
+ */
+
+static int DetectParseTestNoOpt(void)
+{
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    FAIL_IF(DetectEngineAppendSig(de_ctx,
+                    "alert http any any -> any any (msg:\"sid 1 version 0\"; "
+                    "content:\"dummy1\"; endswith: reference: ref; sid:1;)") != NULL);
+    DetectEngineCtxFree(de_ctx);
+
+    PASS;
+}
 
 /**
  * \brief this function registers unit tests for DetectParse
@@ -45,4 +62,5 @@ static int DetectParseTest01 (void)
 void DetectParseRegisterTests(void)
 {
     UtRegisterTest("DetectParseTest01", DetectParseTest01);
+    UtRegisterTest("DetectParseTestNoOpt", DetectParseTestNoOpt);
 }