From: Andreas Herz Date: Mon, 29 Feb 2016 21:37:24 +0000 (+0100) Subject: rule-parsing: quick fix for rules with wrong double quotes X-Git-Tag: suricata-3.0.1RC1~94 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27f6620874b418d7cd5ba43b9852ce0b023435b9;p=thirdparty%2Fsuricata.git rule-parsing: quick fix for rules with wrong double quotes The stripping of leading and trailing "s has issues with rules like the ones described in issue 1638 thus resulted in crashing the rule parser. So for now this is a quick fix which approaches this issue directly by stripping those "s correctly and handling error cases. It also adds the skip for leading spaces at the msg keyword and worksaround a possible null pointer dereference (that should never occur though). A more general approach should be done in the future. --- diff --git a/src/detect-depth.c b/src/detect-depth.c index f58438dae4..9e323f6bf8 100644 --- a/src/detect-depth.c +++ b/src/detect-depth.c @@ -62,12 +62,14 @@ static int DetectDepthSetup (DetectEngineCtx *de_ctx, Signature *s, char *depths SigMatch *pm = NULL; int ret = -1; - /* strip "'s */ - if (depthstr[0] == '\"' && depthstr[strlen(depthstr) - 1] == '\"') { + /* Strip leading and trailing "s. */ + if (depthstr[0] == '\"') { str = SCStrdup(depthstr + 1); if (unlikely(str == NULL)) goto end; - str[strlen(depthstr) - 2] = '\0'; + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(str) - 1] = '\0'; + } dubbed = 1; } diff --git a/src/detect-distance.c b/src/detect-distance.c index a8284dbeb3..33b32787f7 100644 --- a/src/detect-distance.c +++ b/src/detect-distance.c @@ -70,12 +70,14 @@ static int DetectDistanceSetup (DetectEngineCtx *de_ctx, Signature *s, SigMatch *pm = NULL; int ret = -1; - /* strip "'s */ - if (distancestr[0] == '\"' && distancestr[strlen(distancestr) - 1] == '\"') { + /* Strip leading and trailing "s. */ + if (distancestr[0] == '\"') { str = SCStrdup(distancestr + 1); if (unlikely(str == NULL)) goto end; - str[strlen(distancestr) - 2] = '\0'; + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(str) - 1] = '\0'; + } dubbed = 1; } diff --git a/src/detect-l3proto.c b/src/detect-l3proto.c index c67b47c8bb..f7e4995290 100644 --- a/src/detect-l3proto.c +++ b/src/detect-l3proto.c @@ -73,12 +73,14 @@ static int DetectL3ProtoSetup(DetectEngineCtx *de_ctx, Signature *s, char *optst char *str = optstr; char dubbed = 0; - /* strip "'s */ - if (optstr[0] == '\"' && optstr[strlen(optstr) - 1] == '\"') { + /* Strip leading and trailing "s. */ + if (optstr[0] == '\"') { str = SCStrdup(optstr + 1); if (unlikely(str == NULL)) goto error; - str[strlen(optstr) - 2] = '\0'; + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(str) - 1] = '\0'; + } dubbed = 1; } diff --git a/src/detect-msg.c b/src/detect-msg.c index e8b8752967..eddf311972 100644 --- a/src/detect-msg.c +++ b/src/detect-msg.c @@ -51,28 +51,34 @@ static int DetectMsgSetup (DetectEngineCtx *de_ctx, Signature *s, char *msgstr) { char *str = NULL; uint16_t len; + uint16_t pos = 0; + uint16_t slen = 0; - if (strlen(msgstr) == 0) + slen = strlen(msgstr); + if (slen == 0) goto error; - /* strip "'s */ - if (msgstr[0] == '\"' && msgstr[strlen(msgstr)-1] == '\"') { + /* skip the first spaces */ + while (pos < slen && isspace((unsigned char)msgstr[pos])) + pos++; + + /* Strip leading and trailing "s. */ + if (msgstr[pos] == '\"') { str = SCStrdup(msgstr+1); if (unlikely(str == NULL)) goto error; - str[strlen(msgstr)-2] = '\0'; - } else if (msgstr[1] == '\"' && msgstr[strlen(msgstr)-1] == '\"') { - /* XXX do this parsing in a better way */ - str = SCStrdup(msgstr+2); - if (unlikely(str == NULL)) - goto error; - str[strlen(msgstr)-3] = '\0'; - //printf("DetectMsgSetup: format hack applied: \'%s\'\n", str); + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(str)-1] = '\0'; + } } else { SCLogError(SC_ERR_INVALID_VALUE, "format error \'%s\'", msgstr); goto error; } + /* make sure that we don't proceed with null pointer */ + if (unlikely(str == NULL)) + goto error; + len = strlen(str); if (len == 0) goto error; diff --git a/src/detect-offset.c b/src/detect-offset.c index 65372ec0af..e13de7f602 100644 --- a/src/detect-offset.c +++ b/src/detect-offset.c @@ -61,12 +61,14 @@ int DetectOffsetSetup (DetectEngineCtx *de_ctx, Signature *s, char *offsetstr) SigMatch *pm = NULL; int ret = -1; - /* strip "'s */ - if (offsetstr[0] == '\"' && offsetstr[strlen(offsetstr)-1] == '\"') { + /* Strip leading and trailing "s. */ + if (offsetstr[0] == '\"') { str = SCStrdup(offsetstr+1); if (unlikely(str == NULL)) goto end; - str[strlen(offsetstr) - 2] = '\0'; + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(str) - 1] = '\0'; + } dubbed = 1; } diff --git a/src/detect-rev.c b/src/detect-rev.c index 6306f6cc7d..d540ec6163 100644 --- a/src/detect-rev.c +++ b/src/detect-rev.c @@ -46,13 +46,14 @@ static int DetectRevSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr) char *str = rawstr; char dubbed = 0; - /* strip "'s */ - if (rawstr[0] == '\"' && rawstr[strlen(rawstr)-1] == '\"') { + /* Strip leading and trailing "s. */ + if (rawstr[0] == '\"') { str = SCStrdup(rawstr+1); if (unlikely(str == NULL)) return -1; - - str[strlen(rawstr)-2] = '\0'; + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(rawstr)-1] = '\0'; + } dubbed = 1; } diff --git a/src/detect-within.c b/src/detect-within.c index 25c9723867..2845856fe2 100644 --- a/src/detect-within.c +++ b/src/detect-within.c @@ -74,12 +74,14 @@ static int DetectWithinSetup(DetectEngineCtx *de_ctx, Signature *s, char *within SigMatch *pm = NULL; int ret = -1; - /* strip "'s */ - if (withinstr[0] == '\"' && withinstr[strlen(withinstr)-1] == '\"') { + /* Strip leading and trailing "s. */ + if (withinstr[0] == '\"') { str = SCStrdup(withinstr+1); if (unlikely(str == NULL)) goto end; - str[strlen(withinstr) - 2] = '\0'; + if (strlen(str) && str[strlen(str) - 1] == '\"') { + str[strlen(str) - 1] = '\0'; + } dubbed = 1; }