From: Victor Julien Date: Wed, 21 Jan 2015 10:43:58 +0000 (+0100) Subject: detect: fix small memory leaks X-Git-Tag: suricata-2.1beta3~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F1305%2Fhead;p=thirdparty%2Fsuricata.git detect: fix small memory leaks Fix small memory leaks in option parsing. Move away from pcre_get_substring in favor of pcre_copy_substring. Related to #1046. --- diff --git a/src/detect-dce-iface.c b/src/detect-dce-iface.c index 029cebdfb6..60da43ed05 100644 --- a/src/detect-dce-iface.c +++ b/src/detect-dce-iface.c @@ -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; diff --git a/src/detect-engine-event.c b/src/detect-engine-event.c index 737b547899..6b685d7054 100644 --- a/src/detect-engine-event.c +++ b/src/detect-engine-event.c @@ -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; } diff --git a/src/detect-fast-pattern.c b/src/detect-fast-pattern.c index db721562d5..90b0316ad2 100644 --- a/src/detect-fast-pattern.c +++ b/src/detect-fast-pattern.c @@ -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"); diff --git a/src/detect-id.c b/src/detect-id.c index bb084d2c76..5df2a6d182 100644 --- a/src/detect-id.c +++ b/src/detect-id.c @@ -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; } diff --git a/src/detect-priority.c b/src/detect-priority.c index 6db84b28b9..5bb0359f85 100644 --- a/src/detect-priority.c +++ b/src/detect-priority.c @@ -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-------------------------------------*/ diff --git a/src/detect-window.c b/src/detect-window.c index e770f71b1b..0fb1afb865 100644 --- a/src/detect-window.c +++ b/src/detect-window.c @@ -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;