DetectSidSetup - safer stripping of quotes.

Discovered by AFL when using a rule like:
    alert tcp any any -> any any (content:"ABC"; sid:";)
would result in a negative array index.

diff --git a/src/detect-sid.c b/src/detect-sid.c
index 1c7666bb17d20823076704a8ffe6f93e7e3f213a..7e5e1b1f851e650149c3473b3c495801c5452521 100644 (file)
--- a/src/detect-sid.c
+++ b/src/detect-sid.c
@@ -25,10 +25,14 @@
 
 #include "suricata-common.h"
 #include "detect.h"
+#include "detect-engine.h"
+#include "detect-parse.h"
 #include "util-debug.h"
 #include "util-error.h"
+#include "util-unittest.h"
 
 static int DetectSidSetup (DetectEngineCtx *, Signature *, char *);
+static void DetectSidRegisterTests(void);
 
 void DetectSidRegister (void)
 {
@@ -38,7 +42,7 @@ void DetectSidRegister (void)
     sigmatch_table[DETECT_SID].Match = NULL;
     sigmatch_table[DETECT_SID].Setup = DetectSidSetup;
     sigmatch_table[DETECT_SID].Free = NULL;
-    sigmatch_table[DETECT_SID].RegisterTests = NULL;
+    sigmatch_table[DETECT_SID].RegisterTests = DetectSidRegisterTests;
 }
 
 static int DetectSidSetup (DetectEngineCtx *de_ctx, Signature *s, char *sidstr)
@@ -46,13 +50,15 @@ static int DetectSidSetup (DetectEngineCtx *de_ctx, Signature *s, char *sidstr)
     char *str = sidstr;
     char dubbed = 0;
 
-    /* strip "'s */
-    if (sidstr[0] == '\"' && sidstr[strlen(sidstr)-1] == '\"') {
-        str = SCStrdup(sidstr+1);
-        if (unlikely(str == NULL))
+    /* Strip leading and trailing "s. */
+    if (sidstr[0] == '\"') {
+        str = SCStrdup(sidstr + 1);
+        if (unlikely(str == NULL)) {
             return -1;
-
-        str[strlen(sidstr)-2] = '\0';
+        }
+        if (strlen(str) && str[strlen(str) - 1] == '\"') {
+            str[strlen(str) - 1] = '\0';
+        }
         dubbed = 1;
     }
 
@@ -81,3 +87,79 @@ static int DetectSidSetup (DetectEngineCtx *de_ctx, Signature *s, char *sidstr)
     return -1;
 }
 
+#ifdef UNITTESTS
+
+static int SidTestParse01(void)
+{
+    int result = 0;
+    Signature *s = NULL;
+
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
+
+    s = DetectEngineAppendSig(de_ctx,
+        "alert tcp 1.2.3.4 any -> any any (sid:1; gid:1;)");
+    if (s == NULL || s->id != 1)
+        goto end;
+
+    result = 1;
+
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+static int SidTestParse02(void)
+{
+    int result = 0;
+
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
+
+    if (DetectEngineAppendSig(de_ctx,
+            "alert tcp 1.2.3.4 any -> any any (sid:a; gid:1;)") != NULL)
+        goto end;
+
+    result = 1;
+
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+static int SidTestParse03(void)
+{
+    int result = 0;
+
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
+
+    if (DetectEngineAppendSig(de_ctx,
+            "alert tcp any any -> any any (content:\"ABC\"; sid:\";)") != NULL)
+        goto end;
+
+    result = 1;
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+#endif
+
+/**
+ * \brief Register DetectSid unit tests.
+ */
+static void DetectSidRegisterTests(void)
+{
+#ifdef UNITTESTS
+    UtRegisterTest("SidTestParse01", SidTestParse01, 1);
+    UtRegisterTest("SidTestParse02", SidTestParse02, 1);
+    UtRegisterTest("SidTestParse03", SidTestParse03, 1);
+#endif /* UNITTESTS */
+}