Generate proper errors if sid,gid,rev values are out of range. Bug #779.

diff --git a/src/detect-gid.c b/src/detect-gid.c
index 3bd65fc59de3950b2f9d053429a5f6364c97d532..4c918d2d925c5327a94790b1fac101f086ab7f01 100644 (file)
--- a/src/detect-gid.c
+++ b/src/detect-gid.c
@@ -27,6 +27,8 @@
 #include "suricata.h"
 #include "decode.h"
 #include "detect.h"
+#include "detect-engine.h"
+#include "detect-parse.h"
 #include "flow-var.h"
 #include "decode-events.h"
 
@@ -36,9 +38,6 @@
 
 #define PARSE_REGEX "[0-9]+"
 
-static pcre *parse_regex;
-static pcre_extra *parse_regex_study;
-
 static int DetectGidSetup (DetectEngineCtx *, Signature *, char *);
 
 /**
@@ -53,69 +52,6 @@ void DetectGidRegister (void) {
     sigmatch_table[DETECT_GID].Setup = DetectGidSetup;
     sigmatch_table[DETECT_GID].Free  = NULL;
     sigmatch_table[DETECT_GID].RegisterTests = GidRegisterTests;
-
-    const char *eb;
-    int opts = 0;
-    int eo;
-
-    parse_regex = pcre_compile(PARSE_REGEX, opts, &eb, &eo, NULL);
-    if(parse_regex == NULL)
-    {
-        SCLogError(SC_ERR_PCRE_COMPILE, "pcre compile of \"%s\" failed at offset %" PRId32 ": %s", PARSE_REGEX, eo, eb);
-        goto error;
-    }
-
-    parse_regex_study = pcre_study(parse_regex, 0, &eb);
-    if(eb != NULL)
-    {
-        SCLogError(SC_ERR_PCRE_STUDY, "pcre study failed: %s", eb);
-        goto error;
-    }
-
-error:
-    return;
-
-}
-
-/**
- * \internal
- * \brief This function is used to parse gid options passed via gid: keyword
- *
- * \param rawstr Pointer to the user provided gid options
- *
- * \retval  gid number on success
- * \retval -1 on failure
- */
-static uint32_t DetectGidParse (char *rawstr)
-{
-    int ret = 0, res = 0;
-#define MAX_SUBSTRINGS 30
-    int ov[MAX_SUBSTRINGS];
-    const char *str_ptr = NULL;
-    char *ptr = NULL;
-    uint32_t rc;
-
-    ret = pcre_exec(parse_regex, parse_regex_study, rawstr, strlen(rawstr), 0, 0, ov, MAX_SUBSTRINGS);
-    if (ret < 1) {
-        SCLogError(SC_ERR_PCRE_MATCH, "pcre_exec parse error, ret %" PRId32 ", string %s", ret, rawstr);
-        return -1;
-    }
-
-    res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 0, &str_ptr);
-    if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
-        return -1;
-    }
-
-    ptr = (char *)str_ptr;
-
-    if(ptr == NULL)
-        return -1;
-
-    rc = (uint32_t )atol(ptr);
-
-    SCFree(ptr);
-    return rc;
 }
 
 /**
@@ -131,11 +67,41 @@ static uint32_t DetectGidParse (char *rawstr)
  */
 static int DetectGidSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr)
 {
-    s->gid = DetectGidParse(rawstr);
+    char *str = rawstr;
+    char dubbed = 0;
+
+    /* strip "'s */
+    if (rawstr[0] == '\"' && rawstr[strlen(rawstr)-1] == '\"') {
+        str = SCStrdup(rawstr+1);
+        if (unlikely(str == NULL))
+            return -1;
+
+        str[strlen(rawstr)-2] = '\0';
+        dubbed = 1;
+    }
+
+    unsigned long gid = 0;
+    char *endptr = NULL;
+    gid = strtoul(rawstr, &endptr, 10);
+    if (endptr == NULL || *endptr != '\0') {
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid character as arg "
+                   "to gid keyword");
+        goto error;
+    }
+    if (gid >= UINT_MAX) {
+        SCLogError(SC_ERR_INVALID_NUMERIC_VALUE, "gid value to high, max %u", UINT_MAX);
+        goto error;
+    }
 
-    if(s->gid > 0)
-        return 0;
+    s->gid = (uint32_t)gid;
 
+    if (dubbed)
+        SCFree(str);
+    return 0;
+
+ error:
+    if (dubbed)
+        SCFree(str);
     return -1;
 }
 
@@ -151,16 +117,22 @@ static int DetectGidSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr)
  *  \retval 0 on failure
  */
 static int GidTestParse01 (void) {
-
-    int gid = 0;
-
-    gid = DetectGidParse("1");
-
-    if (gid == 1) {
-        return 1;
-    }
-
-    return 0;
+    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->gid != 1)
+        goto end;
+
+    result = 1;
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
 }
 
 /**
@@ -170,16 +142,20 @@ static int GidTestParse01 (void) {
  *  \retval 0 on failure
  */
 static int GidTestParse02 (void) {
+    int result = 0;
 
-    int gid = 0;
+    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
+    if (de_ctx == NULL)
+        goto end;
 
-    gid = DetectGidParse("a");
+    if (DetectEngineAppendSig(de_ctx, "alert tcp 1.2.3.4 any -> any any (sid:1; gid:a;)") != NULL)
+        goto end;
 
-    if (gid > 1) {
-        return 1;
-    }
-
-    return 0;
+    result = 1;
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
 }
 #endif /* UNITTESTS */
 
@@ -189,6 +165,6 @@ static int GidTestParse02 (void) {
 void GidRegisterTests(void) {
 #ifdef UNITTESTS
     UtRegisterTest("GidTestParse01", GidTestParse01, 1);
-    UtRegisterTest("GidTestParse02", GidTestParse02, 0);
+    UtRegisterTest("GidTestParse02", GidTestParse02, 1);
 #endif /* UNITTESTS */
 }

diff --git a/src/detect-parse.c b/src/detect-parse.c
index c3ba7bf1c4915d3d23a1d8f2d4f084869344845b..804a7a74d10438ea47657d686b05eb3dd814584f 100644 (file)
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -2277,6 +2277,60 @@ end:
     return result;
 }
 
+/** \test sid value too large. Bug #779 */
+static int SigParseTest18 (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 -> !1.2.3.4 any (msg:\"SigParseTest01\"; sid:99999999999999999999;)") != NULL)
+        goto end;
+
+    result = 1;
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+/** \test gid value too large. Related to bug #779 */
+static int SigParseTest19 (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 -> !1.2.3.4 any (msg:\"SigParseTest01\"; sid:1; gid:99999999999999999999;)") != NULL)
+        goto end;
+
+    result = 1;
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
+/** \test rev value too large. Related to bug #779 */
+static int SigParseTest20 (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 -> !1.2.3.4 any (msg:\"SigParseTest01\"; sid:1; rev:99999999999999999999;)") != NULL)
+        goto end;
+
+    result = 1;
+end:
+    if (de_ctx != NULL)
+        DetectEngineCtxFree(de_ctx);
+    return result;
+}
+
 /** \test Direction operator validation (invalid) */
 int SigParseBidirecTest06 (void) {
     int result = 1;
@@ -3156,6 +3210,9 @@ void SigParseRegisterTests(void) {
     UtRegisterTest("SigParseTest15", SigParseTest15, 1);
     UtRegisterTest("SigParseTest16", SigParseTest16, 1);
     UtRegisterTest("SigParseTest17", SigParseTest17, 1);
+    UtRegisterTest("SigParseTest18", SigParseTest18, 1);
+    UtRegisterTest("SigParseTest19", SigParseTest19, 1);
+    UtRegisterTest("SigParseTest20", SigParseTest20, 1);
 
     UtRegisterTest("SigParseBidirecTest06", SigParseBidirecTest06, 1);
     UtRegisterTest("SigParseBidirecTest07", SigParseBidirecTest07, 1);

diff --git a/src/detect-rev.c b/src/detect-rev.c
index 94e90e204a9782972a21a83972f525fc88049794..5496200321b64dac4cd0a2d5b1d09655c42cda60 100644 (file)
--- a/src/detect-rev.c
+++ b/src/detect-rev.c
@@ -59,10 +59,15 @@ static int DetectRevSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr)
     char *endptr = NULL;
     rev = strtoul(rawstr, &endptr, 10);
     if (endptr == NULL || *endptr != '\0') {
-        SCLogError(SC_ERR_INVALID_SIGNATURE, "Saw an invalid character as arg "
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid character as arg "
                    "to rev keyword");
         goto error;
     }
+    if (rev >= UINT_MAX) {
+        SCLogError(SC_ERR_INVALID_NUMERIC_VALUE, "rev value to high, max %u", UINT_MAX);
+        goto error;
+    }
+
     s->rev = (uint32_t)rev;
 
     if (dubbed)

diff --git a/src/detect-sid.c b/src/detect-sid.c
index 7d2f15eb044c22542586c01b54c8ea6e639e4a7c..452d83572a57e3607519d2b1bb6c8caafe2e77a1 100644 (file)
--- a/src/detect-sid.c
+++ b/src/detect-sid.c
@@ -59,10 +59,15 @@ static int DetectSidSetup (DetectEngineCtx *de_ctx, Signature *s, char *sidstr)
     char *endptr = NULL;
     id = strtoul(sidstr, &endptr, 10);
     if (endptr == NULL || *endptr != '\0') {
-        SCLogError(SC_ERR_INVALID_SIGNATURE, "Saw an invalid character as arg "
+        SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid character as arg "
                    "to sid keyword");
         goto error;
     }
+    if (id >= UINT_MAX) {
+        SCLogError(SC_ERR_INVALID_NUMERIC_VALUE, "sid value to high, max %u", UINT_MAX);
+        goto error;
+    }
+
     s->id = (uint32_t)id;
 
     if (dubbed)