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.
pull/1305/head
Victor Julien 11 years ago
parent 5a8094136c
commit 5b6f8bda1d

@ -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;

@ -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;
}

@ -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");

@ -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;
}

@ -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-------------------------------------*/

@ -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;

Loading…
Cancel
Save