]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
threshold: Change rule parsing to use pcre_copy_substring
authorCarl Smith <carl.smith@alliedtelesis.co.nz>
Sun, 16 Aug 2020 20:41:35 +0000 (08:41 +1200)
committerVictor Julien <victor@inliniac.net>
Mon, 24 Aug 2020 14:03:25 +0000 (16:03 +0200)
Fixes memory leak when parsing threshold rules.
All parsed strings are less than 16 characters except
for the IP address which could be up to 48 characters.
Remove redefinition of MAX_SUBSTRINGS

src/util-threshold-config.c

index 55463387f4141ffa8233950c0860cfca227444ed..8b57ff7f75a0d6516326734c247c39b6bad90102 100644 (file)
@@ -649,13 +649,13 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
     char th_gid[16];
     char th_sid[16];
     char rule_extend[1024];
-    const char *th_type = NULL;
-    const char *th_track = NULL;
-    const char *th_count = NULL;
-    const char *th_seconds = NULL;
-    const char *th_new_action= NULL;
-    const char *th_timeout = NULL;
-    const char *th_ip = NULL;
+    char th_type[16] = "";
+    char th_track[16] = "";
+    char th_count[16] = "";
+    char th_seconds[16] = "";
+    char th_new_action[16] = "";
+    char th_timeout[16] = "";
+    char th_ip[64] = "";
 
     uint8_t parsed_type = 0;
     uint8_t parsed_track = 0;
@@ -664,7 +664,6 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
     uint32_t parsed_seconds = 0;
     uint32_t parsed_timeout = 0;
 
-#define MAX_SUBSTRINGS 30
     int ret = 0;
     int ov[MAX_SUBSTRINGS];
     uint32_t id = 0, gid = 0;
@@ -680,28 +679,28 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
     }
 
     /* retrieve the classtype name */
-    ret = pcre_copy_substring((char *)rawstr, ov, 30, 1, th_rule_type, sizeof(th_rule_type));
+    ret = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 1, th_rule_type, sizeof(th_rule_type));
     if (ret < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
+        SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
         goto error;
     }
 
     /* retrieve the classtype name */
-    ret = pcre_copy_substring((char *)rawstr, ov, 30, 2, th_gid, sizeof(th_gid));
+    ret = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 2, th_gid, sizeof(th_gid));
     if (ret < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
+        SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
         goto error;
     }
 
-    ret = pcre_copy_substring((char *)rawstr, ov, 30, 3, th_sid, sizeof(th_sid));
+    ret = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 3, th_sid, sizeof(th_sid));
     if (ret < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
+        SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
         goto error;
     }
 
-    ret = pcre_copy_substring((char *)rawstr, ov, 30, 4, rule_extend, sizeof(rule_extend));
+    ret = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 4, rule_extend, sizeof(rule_extend));
     if (ret < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
+        SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
         goto error;
     }
 
@@ -734,27 +733,27 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, 30, 1, &th_type);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 1, th_type, sizeof(th_type));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, 30, 2, &th_track);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 2, th_track, sizeof(th_track));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, 30, 3, &th_count);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 3, th_count, sizeof(th_count));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, 30, 4, &th_seconds);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 4, th_seconds, sizeof(th_seconds));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
@@ -785,15 +784,15 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
                     goto error;
                 }
                 /* retrieve the track mode */
-                ret = pcre_get_substring((char *)rule_extend, ov, 30, 1, &th_track);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 1, th_track, sizeof(th_track));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
                 /* retrieve the IP */
-                ret = pcre_get_substring((char *)rule_extend, ov, 30, 2, &th_ip);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 2, th_ip, sizeof(th_ip));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
             } else {
@@ -813,33 +812,33 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 1, &th_track);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 1, th_track, sizeof(th_track));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 2, &th_count);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 2, th_count, sizeof(th_count));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 3, &th_seconds);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 3, th_seconds, sizeof(th_seconds));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 4, &th_new_action);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 4, th_new_action, sizeof(th_new_action));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
-                ret = pcre_get_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 5, &th_timeout);
+                ret = pcre_copy_substring((char *)rule_extend, ov, MAX_SUBSTRINGS, 5, th_timeout, sizeof(th_timeout));
                 if (ret < 0) {
-                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                    SCLogError(SC_ERR_PCRE_COPY_SUBSTRING, "pcre_copy_substring failed");
                     goto error;
                 }
 
@@ -908,7 +907,7 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
            break;
         case THRESHOLD_TYPE_SUPPRESS:
             /* need to get IP if extension is provided */
-            if (th_track != NULL) {
+            if (strcmp("", th_track) != 0) {
                 if (strcasecmp(th_track,"by_dst") == 0)
                     parsed_track = TRACK_DST;
                 else if (strcasecmp(th_track,"by_src") == 0)
@@ -940,19 +939,14 @@ static int ParseThresholdRule(DetectEngineCtx *de_ctx, char *rawstr,
     *ret_parsed_count = parsed_count;
     *ret_parsed_seconds = parsed_seconds;
     *ret_parsed_timeout = parsed_timeout;
-    *ret_th_ip = th_ip;
+    *ret_th_ip = NULL;
+    if (strcmp("", th_ip) != 0) {
+        *ret_th_ip = SCStrdup(th_ip);
+        if (*ret_th_ip == NULL)
+            goto error;
+    }
     return 0;
 error:
-    if (th_track != NULL)
-        SCFree((char *)th_track);
-    if (th_count != NULL)
-        SCFree((char *)th_count);
-    if (th_seconds != NULL)
-        SCFree((char *)th_seconds);
-    if (th_type != NULL)
-        SCFree((char *)th_type);
-    if (th_ip != NULL)
-        SCFree((char *)th_ip);
     return -1;
 }