]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect: fix small memory leaks 1305/head
authorVictor Julien <victor@inliniac.net>
Wed, 21 Jan 2015 10:43:58 +0000 (11:43 +0100)
committerVictor Julien <victor@inliniac.net>
Tue, 27 Jan 2015 08:16:43 +0000 (09:16 +0100)
Fix small memory leaks in option parsing. Move away from
pcre_get_substring in favor of pcre_copy_substring.

Related to #1046.

src/detect-dce-iface.c
src/detect-engine-event.c
src/detect-fast-pattern.c
src/detect-id.c
src/detect-priority.c
src/detect-window.c

index 029cebdfb63f06196937e8d7ad805eaa7043446e..60da43ed05a66c026c332fbb096d77f8db4b5c24 100644 (file)
@@ -113,10 +113,10 @@ static inline DetectDceIfaceData *DetectDceIfaceArgParse(const char *arg)
     int ret = 0, res = 0;
     int ov[MAX_SUBSTRINGS];
     uint8_t hex_value;
-    const char *pcre_sub_str = NULL;
+    char copy_str[128] = "";
     int i = 0, j = 0;
     int len = 0;
-    char temp_str[3];
+    char temp_str[3] = "";
     int version;
 
     ret = pcre_exec(parse_regex, parse_regex_study, arg, strlen(arg), 0, 0, ov,
@@ -131,24 +131,24 @@ static inline DetectDceIfaceData *DetectDceIfaceArgParse(const char *arg)
     memset(did, 0, sizeof(DetectDceIfaceData));
 
     /* retrieve the iface uuid string.  iface uuid is a compulsion in the keyword */
-    res = pcre_get_substring(arg, ov, MAX_SUBSTRINGS, 1, &pcre_sub_str);
+    res = pcre_copy_substring(arg, ov, MAX_SUBSTRINGS, 1, copy_str, sizeof(copy_str));
     if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
         goto error;
     }
 
     /* parse the iface uuid string */
-    len = strlen(pcre_sub_str);
+    len = strlen(copy_str);
     j = 0;
     temp_str[2] = '\0';
     for (i = 0; i < len; ) {
-        if (pcre_sub_str[i] == '-') {
+        if (copy_str[i] == '-') {
             i++;
             continue;
         }
 
-        temp_str[0] = pcre_sub_str[i];
-        temp_str[1] = pcre_sub_str[i + 1];
+        temp_str[0] = copy_str[i];
+        temp_str[1] = copy_str[i + 1];
 
         hex_value = strtol(temp_str, NULL, 16);
         did->uuid[j] = hex_value;
@@ -164,13 +164,13 @@ static inline DetectDceIfaceData *DetectDceIfaceArgParse(const char *arg)
     if (ret == 4 || ret == 5) {
         /* first handle the version number, so that we can do some additional
          * validations of the version number, wrt. the operator */
-        res = pcre_get_substring(arg, ov, MAX_SUBSTRINGS, 3, &pcre_sub_str);
+        res = pcre_copy_substring(arg, ov, MAX_SUBSTRINGS, 3, copy_str, sizeof(copy_str));
         if (res < 0) {
-            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
             goto error;
         }
 
-        version = atoi(pcre_sub_str);
+        version = atoi(copy_str);
         if (version > UINT16_MAX) {
             SCLogError(SC_ERR_INVALID_SIGNATURE, "DCE_IFACE interface version "
                        "invalid: %d\n", version);
@@ -178,17 +178,14 @@ static inline DetectDceIfaceData *DetectDceIfaceArgParse(const char *arg)
         }
         did->version = version;
 
-        /* free the substring */
-        pcre_free_substring(pcre_sub_str);
-
         /* now let us handle the operator supplied with the version number */
-        res = pcre_get_substring(arg, ov, MAX_SUBSTRINGS, 2, &pcre_sub_str);
+        res = pcre_copy_substring(arg, ov, MAX_SUBSTRINGS, 2, copy_str, sizeof(copy_str));
         if (res < 0) {
-            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
             goto error;
         }
 
-        switch (pcre_sub_str[0]) {
+        switch (copy_str[0]) {
             case '<':
                 if (version == 0) {
                     SCLogError(SC_ERR_INVALID_SIGNATURE, "DCE_IFACE interface "
@@ -217,9 +214,6 @@ static inline DetectDceIfaceData *DetectDceIfaceArgParse(const char *arg)
                 did->op = DETECT_DCE_IFACE_OP_NE;
                 break;
         }
-
-        /* free the substring */
-        pcre_free_substring(pcre_sub_str);
     }
 
     return did;
index 737b5478998da715560436db05b667f615acc4d9..6b685d70543e7795b328b3e50c9f6b46b41c3c2b 100644 (file)
@@ -151,16 +151,17 @@ DetectEngineEventData *DetectEngineEventParse (char *rawstr)
         goto error;
     }
 
-    const char *str_ptr;
-    res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 0, &str_ptr);
+    char copy_str[128] = "";
+    res = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 0,
+            copy_str, sizeof(copy_str));
 
     if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
         goto error;
     }
 
     for (i = 0; DEvents[i].event_name != NULL; i++) {
-        if (strcasecmp(DEvents[i].event_name,str_ptr) == 0) {
+        if (strcasecmp(DEvents[i].event_name,copy_str) == 0) {
             found = 1;
             break;
         }
@@ -168,7 +169,7 @@ DetectEngineEventData *DetectEngineEventParse (char *rawstr)
 
     if (found == 0) {
         SCLogError(SC_ERR_UNKNOWN_DECODE_EVENT, "unknown decode event \"%s\"",
-                str_ptr);
+                copy_str);
         goto error;
     }
 
@@ -184,7 +185,8 @@ DetectEngineEventData *DetectEngineEventParse (char *rawstr)
     return de;
 
 error:
-    if (de) SCFree(de);
+    if (de)
+        SCFree(de);
     return NULL;
 }
 
index db721562d5f0b71ab94e258d85e1a3082403293c..90b0316ad23f2b6226785eb32a24437c451fd9c4 100644 (file)
@@ -208,7 +208,7 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, char *a
 #define MAX_SUBSTRINGS 30
     int ret = 0, res = 0;
     int ov[MAX_SUBSTRINGS];
-    const char *arg_substr = NULL;
+    char arg_substr[128] = "";
     DetectContentData *cd = NULL;
 
     if (s->sm_lists_tail[DETECT_SM_LIST_PMATCH] == NULL &&
@@ -320,8 +320,8 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, char *a
 
         /* fast pattern chop */
     } else if (ret == 4) {
-        res = pcre_get_substring((char *)arg, ov, MAX_SUBSTRINGS,
-                                 2, &arg_substr);
+        res = pcre_copy_substring((char *)arg, ov, MAX_SUBSTRINGS,
+                                 2, arg_substr, sizeof(arg_substr));
         if (res < 0) {
             SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed "
                        "for fast_pattern offset");
@@ -334,8 +334,8 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, char *a
             goto error;
         }
 
-        res = pcre_get_substring((char *)arg, ov, MAX_SUBSTRINGS,
-                                 3, &arg_substr);
+        res = pcre_copy_substring((char *)arg, ov, MAX_SUBSTRINGS,
+                                 3, arg_substr, sizeof(arg_substr));
         if (res < 0) {
             SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed "
                        "for fast_pattern offset");
index bb084d2c767133a346421364eb207598f78d7fa8..5df2a6d1823ec99c02e3203c022d6a6b535d0d8e 100644 (file)
@@ -139,7 +139,6 @@ DetectIdData *DetectIdParse (char *idstr)
     int ret = 0, res = 0;
     int ov[MAX_SUBSTRINGS];
 
-
     ret = pcre_exec(parse_regex, parse_regex_study, idstr, strlen(idstr), 0, 0,
                     ov, MAX_SUBSTRINGS);
 
@@ -152,26 +151,16 @@ DetectIdData *DetectIdParse (char *idstr)
 
 
     if (ret > 1) {
-        const char *str_ptr;
-        char *orig;
+        char copy_str[128] = "";
         char *tmp_str;
-        res = pcre_get_substring((char *)idstr, ov, MAX_SUBSTRINGS, 1,
-                                    &str_ptr);
+        res = pcre_copy_substring((char *)idstr, ov, MAX_SUBSTRINGS, 1,
+                                    copy_str, sizeof(copy_str));
         if (res < 0) {
-            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
             goto error;
         }
 
-        /* We have a correct id option */
-        id_d = SCMalloc(sizeof(DetectIdData));
-        if (unlikely(id_d == NULL))
-            goto error;
-
-        orig = SCStrdup((char*)str_ptr);
-        if (unlikely(orig == NULL)) {
-            goto error;
-        }
-        tmp_str=orig;
+        tmp_str = copy_str;
 
         /* Let's see if we need to scape "'s */
         if (tmp_str[0] == '"')
@@ -187,13 +176,15 @@ DetectIdData *DetectIdParse (char *idstr)
             SCLogError(SC_ERR_INVALID_VALUE, "\"id\" option  must be in "
                         "the range %u - %u",
                         DETECT_IPID_MIN, DETECT_IPID_MAX);
-
-            SCFree(orig);
             goto error;
         }
-        id_d->id = temp;
 
-        SCFree(orig);
+        /* We have a correct id option */
+        id_d = SCMalloc(sizeof(DetectIdData));
+        if (unlikely(id_d == NULL))
+            goto error;
+
+        id_d->id = temp;
 
         SCLogDebug("detect-id: will look for ip_id: %u\n", id_d->id);
     }
@@ -201,7 +192,8 @@ DetectIdData *DetectIdParse (char *idstr)
     return id_d;
 
 error:
-    if (id_d != NULL) DetectIdFree(id_d);
+    if (id_d != NULL)
+        DetectIdFree(id_d);
     return NULL;
 
 }
index 6db84b28b94983e950b0fa56f734421148d5d8d7..5bb0359f85cae48c2ada9ef7799c5d3be6cb2552 100644 (file)
@@ -77,7 +77,7 @@ void DetectPriorityRegister (void)
 
 static int DetectPrioritySetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr)
 {
-    const char *prio_str = NULL;
+    char copy_str[128] = "";
 
 #define MAX_SUBSTRINGS 30
     int ret = 0;
@@ -90,26 +90,24 @@ static int DetectPrioritySetup (DetectEngineCtx *de_ctx, Signature *s, char *raw
         return -1;
     }
 
-    ret = pcre_get_substring((char *)rawstr, ov, 30, 1, &prio_str);
+    ret = pcre_copy_substring((char *)rawstr, ov, 30, 1, copy_str, sizeof(copy_str));
     if (ret < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
         return -1;
     }
 
     long prio = 0;
     char *endptr = NULL;
-    prio = strtol(rawstr, &endptr, 10);
+    prio = strtol(copy_str, &endptr, 10);
     if (endptr == NULL || *endptr != '\0') {
         SCLogError(SC_ERR_INVALID_SIGNATURE, "Saw an invalid character as arg "
                    "to priority keyword");
-        goto error;
+        return -1;
     }
     /* if we have reached here, we have had a valid priority.  Assign it */
     s->prio = prio;
 
     return 0;
- error:
-    return -1;
 }
 
 /*------------------------------Unittests-------------------------------------*/
index e770f71b1be26633386139f76b7922d5540d970e..0fb1afb865017126eb47233b04ff4e468b2dd6b9 100644 (file)
@@ -130,15 +130,11 @@ int DetectWindowMatch(ThreadVars *t, DetectEngineThreadCtx *det_ctx, Packet *p,
 DetectWindowData *DetectWindowParse(char *windowstr)
 {
     DetectWindowData *wd = NULL;
-    char *args[3] = {NULL,NULL,NULL}; /* PR: Why PCRE MAX_SUBSTRING must be multiple of 3? */
-       #define MAX_SUBSTRINGS 30
-
+#define MAX_SUBSTRINGS 30
     int ret = 0, res = 0;
     int ov[MAX_SUBSTRINGS];
 
-
     ret = pcre_exec(parse_regex, parse_regex_study, windowstr, strlen(windowstr), 0, 0, ov, MAX_SUBSTRINGS);
-
     if (ret < 1 || ret > 3) {
         SCLogError(SC_ERR_PCRE_MATCH, "pcre_exec parse error, ret %" PRId32 ", string %s", ret, windowstr);
         goto error;
@@ -149,45 +145,39 @@ DetectWindowData *DetectWindowParse(char *windowstr)
         goto error;
 
     if (ret > 1) {
-        const char *str_ptr;
-        res = pcre_get_substring((char *)windowstr, ov, MAX_SUBSTRINGS, 1, &str_ptr);
+        char copy_str[128] = "";
+        res = pcre_copy_substring((char *)windowstr, ov, MAX_SUBSTRINGS, 1,
+                copy_str, sizeof(copy_str));
         if (res < 0) {
-            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
             goto error;
         }
-        args[0] = (char *)str_ptr;
+
         /* Detect if it's negated */
-        if (args[0][0] == '!')
+        if (copy_str[0] == '!')
             wd->negated = 1;
         else
             wd->negated = 0;
 
         if (ret > 2) {
-            res = pcre_get_substring((char *)windowstr, ov, MAX_SUBSTRINGS, 2, &str_ptr);
+            res = pcre_copy_substring((char *)windowstr, ov, MAX_SUBSTRINGS, 2,
+                    copy_str, sizeof(copy_str));
             if (res < 0) {
-                SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
                 goto error;
             }
 
-            /* Get the window size if it's a valid value (in packets, we should alert if this doesn't happend from decode) */
-            if (-1 == ByteExtractStringUint16(&wd->size, 10, 0, str_ptr)) {
+            /* Get the window size if it's a valid value (in packets, we
+             * should alert if this doesn't happend from decode) */
+            if (-1 == ByteExtractStringUint16(&wd->size, 10, 0, copy_str)) {
                 goto error;
             }
         }
     }
 
-       int i = 0;
-    for (i = 0; i < (ret -1); i++){
-        if (args[i] != NULL)
-            SCFree(args[i]);
-    }
     return wd;
 
 error:
-    for (i = 0; i < (ret -1) && i < 3; i++){
-        if (args[i] != NULL)
-            SCFree(args[i]);
-    }
     if (wd != NULL)
         DetectWindowFree(wd);
     return NULL;