From 81d7a7aa826f96deceddcae83bcdae0c925b3dab Mon Sep 17 00:00:00 2001 From: Carl Smith Date: Mon, 17 Aug 2020 08:41:35 +1200 Subject: [PATCH] threshold: Change rule parsing to use pcre_copy_substring 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 | 94 +++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 50 deletions(-) diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c index 55463387f4..8b57ff7f75 100644 --- a/src/util-threshold-config.c +++ b/src/util-threshold-config.c @@ -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; }