From 0a39e0653554184df4ab0e08700df81051fe1233 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 25 Oct 2018 12:30:12 +0200 Subject: [PATCH] detect/flags: cleanup parsing to not alloc temp strings --- src/detect-flags.c | 271 +++++++++++++++++++++------------------------ 1 file changed, 128 insertions(+), 143 deletions(-) diff --git a/src/detect-flags.c b/src/detect-flags.c index d5e9145df2..4eceee086e 100644 --- a/src/detect-flags.c +++ b/src/detect-flags.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2010 Open Information Security Foundation +/* Copyright (C) 2007-2018 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -167,127 +167,57 @@ static DetectFlagsData *DetectFlagsParse (const char *rawstr) { SCEnter(); - DetectFlagsData *de = NULL; #define MAX_SUBSTRINGS 30 int ret = 0, found = 0, ignore = 0, res = 0; int ov[MAX_SUBSTRINGS]; - const char *str_ptr = NULL; - char *args[3] = { NULL, NULL, NULL }; char *ptr; - int i; + + char arg1[16] = ""; + char arg2[16] = ""; + char arg3[16] = ""; ret = pcre_exec(parse_regex, parse_regex_study, rawstr, strlen(rawstr), 0, 0, ov, MAX_SUBSTRINGS); - if (ret < 1) { + SCLogDebug("input '%s', pcre said %d", rawstr, ret); + if (ret < 3) { SCLogError(SC_ERR_PCRE_MATCH, "pcre match failed"); - goto error; + SCReturnPtr(NULL, "DetectFlagsData"); } - for (i = 0; i < (ret - 1); i++) { - - res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS,i + 1, - &str_ptr); + res = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 1, arg1, sizeof(arg1)); + if (res < 0) { + SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed"); + SCReturnPtr(NULL, "DetectFlagsData"); + } + if (ret >= 2) { + res = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 2, arg2, sizeof(arg2)); if (res < 0) { SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed"); - goto error; + SCReturnPtr(NULL, "DetectFlagsData"); + } + } + if (ret >= 3) { + res = pcre_copy_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 3, arg3, sizeof(arg3)); + if (res < 0) { + SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed"); + SCReturnPtr(NULL, "DetectFlagsData"); } - - args[i] = (char *)str_ptr; } + SCLogDebug("args '%s', '%s', '%s'", arg1, arg2, arg3); - if(args[1] == NULL) { - SCLogDebug("args[1] == NULL"); - goto error; + if (strlen(arg2) == 0) { + SCLogDebug("empty argument"); + SCReturnPtr(NULL, "DetectFlagsData"); } - de = SCMalloc(sizeof(DetectFlagsData)); + DetectFlagsData *de = SCMalloc(sizeof(DetectFlagsData)); if (unlikely(de == NULL)) goto error; - - memset(de,0,sizeof(DetectFlagsData)); - + memset(de, 0, sizeof(DetectFlagsData)); de->ignored_flags = 0xff; - /** First parse args[0] */ - - if(args[0]) { - - ptr = args[0]; - - while (*ptr != '\0') { - switch (*ptr) { - case 'S': - case 's': - de->flags |= TH_SYN; - found++; - break; - case 'A': - case 'a': - de->flags |= TH_ACK; - found++; - break; - case 'F': - case 'f': - de->flags |= TH_FIN; - found++; - break; - case 'R': - case 'r': - de->flags |= TH_RST; - found++; - break; - case 'P': - case 'p': - de->flags |= TH_PUSH; - found++; - break; - case 'U': - case 'u': - de->flags |= TH_URG; - found++; - break; - case '1': - de->flags |= TH_CWR; - found++; - break; - case '2': - de->flags |= TH_ECN; - found++; - break; - case 'C': - case 'c': - de->flags |= TH_CWR; - found++; - break; - case 'E': - case 'e': - de->flags |= TH_ECN; - found++; - break; - case '0': - de->flags = 0; - found++; - break; - - case '!': - de->modifier = MODIFIER_NOT; - break; - case '+': - de->modifier = MODIFIER_PLUS; - break; - case '*': - de->modifier = MODIFIER_ANY; - break; - } - ptr++; - } - - } - - /** Second parse first set of flags */ - - ptr = args[1]; - + /** First parse args1 */ + ptr = arg1; while (*ptr != '\0') { switch (*ptr) { case 'S': @@ -344,46 +274,110 @@ static DetectFlagsData *DetectFlagsParse (const char *rawstr) break; case '!': - if (de->modifier != 0) { - SCLogError(SC_ERR_FLAGS_MODIFIER, "\"flags\" supports only" - " one modifier at a time"); - goto error; - } de->modifier = MODIFIER_NOT; - SCLogDebug("NOT modifier is set"); break; case '+': - if (de->modifier != 0) { - SCLogError(SC_ERR_FLAGS_MODIFIER, "\"flags\" supports only" - " one modifier at a time"); - goto error; - } de->modifier = MODIFIER_PLUS; - SCLogDebug("PLUS modifier is set"); break; case '*': - if (de->modifier != 0) { - SCLogError(SC_ERR_FLAGS_MODIFIER, "\"flags\" supports only" - " one modifier at a time"); - goto error; - } de->modifier = MODIFIER_ANY; - SCLogDebug("ANY modifier is set"); - break; - default: break; } ptr++; } - if(found == 0) - goto error; + /** Second parse first set of flags */ + if (strlen(arg2) > 0) { + ptr = arg2; + while (*ptr != '\0') { + switch (*ptr) { + case 'S': + case 's': + de->flags |= TH_SYN; + found++; + break; + case 'A': + case 'a': + de->flags |= TH_ACK; + found++; + break; + case 'F': + case 'f': + de->flags |= TH_FIN; + found++; + break; + case 'R': + case 'r': + de->flags |= TH_RST; + found++; + break; + case 'P': + case 'p': + de->flags |= TH_PUSH; + found++; + break; + case 'U': + case 'u': + de->flags |= TH_URG; + found++; + break; + case '1': + case 'C': + case 'c': + de->flags |= TH_CWR; + found++; + break; + case '2': + case 'E': + case 'e': + de->flags |= TH_ECN; + found++; + break; + case '0': + de->flags = 0; + found++; + break; - /** Finally parse ignored flags */ + case '!': + if (de->modifier != 0) { + SCLogError(SC_ERR_FLAGS_MODIFIER, "\"flags\" supports only" + " one modifier at a time"); + goto error; + } + de->modifier = MODIFIER_NOT; + SCLogDebug("NOT modifier is set"); + break; + case '+': + if (de->modifier != 0) { + SCLogError(SC_ERR_FLAGS_MODIFIER, "\"flags\" supports only" + " one modifier at a time"); + goto error; + } + de->modifier = MODIFIER_PLUS; + SCLogDebug("PLUS modifier is set"); + break; + case '*': + if (de->modifier != 0) { + SCLogError(SC_ERR_FLAGS_MODIFIER, "\"flags\" supports only" + " one modifier at a time"); + goto error; + } + de->modifier = MODIFIER_ANY; + SCLogDebug("ANY modifier is set"); + break; + default: + break; + } + ptr++; + } - if(args[2]) { + if (found == 0) + goto error; + } - ptr = args[2]; + /** Finally parse ignored flags */ + if (strlen(arg3) > 0) { + ptr = arg3; while (*ptr != '\0') { switch (*ptr) { @@ -443,25 +437,19 @@ static DetectFlagsData *DetectFlagsParse (const char *rawstr) ptr++; } - if(ignore == 0) { + if (ignore == 0) { SCLogDebug("ignore == 0"); goto error; } } - for (i = 0; i < (ret - 1); i++){ - SCLogDebug("args[%"PRId32"] = %s",i, args[i]); - if (args[i] != NULL) SCFree(args[i]); - } - SCLogDebug("found %"PRId32" ignore %"PRId32"", found, ignore); SCReturnPtr(de, "DetectFlagsData"); error: - for (i = 0; i < (ret - 1); i++){ - if (args[i] != NULL) SCFree(args[i]); + if (de) { + SCFree(de); } - if (de) SCFree(de); SCReturnPtr(NULL, "DetectFlagsData"); } @@ -628,14 +616,11 @@ static _Bool PrefilterTcpFlagsIsPrefilterable(const Signature *s) */ static int FlagsTestParse01 (void) { - DetectFlagsData *de = NULL; - de = DetectFlagsParse("S"); - if (de && (de->flags == TH_SYN) ) { - DetectFlagsFree(de); - return 1; - } - - return 0; + DetectFlagsData *de = DetectFlagsParse("S"); + FAIL_IF_NULL(de); + FAIL_IF_NOT(de->flags == TH_SYN); + DetectFlagsFree(de); + PASS; } /**