From: Victor Julien Date: Tue, 9 Jul 2013 16:36:54 +0000 (+0200) Subject: Generate proper errors if sid,gid,rev values are out of range. Bug #779. X-Git-Tag: suricata-2.0beta1~32 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=73e27c1fb7f3dcff802a41f73125c2287cc2f5a3;p=thirdparty%2Fsuricata.git Generate proper errors if sid,gid,rev values are out of range. Bug #779. --- diff --git a/src/detect-gid.c b/src/detect-gid.c index 3bd65fc59d..4c918d2d92 100644 --- a/src/detect-gid.c +++ b/src/detect-gid.c @@ -27,6 +27,8 @@ #include "suricata.h" #include "decode.h" #include "detect.h" +#include "detect-engine.h" +#include "detect-parse.h" #include "flow-var.h" #include "decode-events.h" @@ -36,9 +38,6 @@ #define PARSE_REGEX "[0-9]+" -static pcre *parse_regex; -static pcre_extra *parse_regex_study; - static int DetectGidSetup (DetectEngineCtx *, Signature *, char *); /** @@ -53,69 +52,6 @@ void DetectGidRegister (void) { sigmatch_table[DETECT_GID].Setup = DetectGidSetup; sigmatch_table[DETECT_GID].Free = NULL; sigmatch_table[DETECT_GID].RegisterTests = GidRegisterTests; - - const char *eb; - int opts = 0; - int eo; - - parse_regex = pcre_compile(PARSE_REGEX, opts, &eb, &eo, NULL); - if(parse_regex == NULL) - { - SCLogError(SC_ERR_PCRE_COMPILE, "pcre compile of \"%s\" failed at offset %" PRId32 ": %s", PARSE_REGEX, eo, eb); - goto error; - } - - parse_regex_study = pcre_study(parse_regex, 0, &eb); - if(eb != NULL) - { - SCLogError(SC_ERR_PCRE_STUDY, "pcre study failed: %s", eb); - goto error; - } - -error: - return; - -} - -/** - * \internal - * \brief This function is used to parse gid options passed via gid: keyword - * - * \param rawstr Pointer to the user provided gid options - * - * \retval gid number on success - * \retval -1 on failure - */ -static uint32_t DetectGidParse (char *rawstr) -{ - int ret = 0, res = 0; -#define MAX_SUBSTRINGS 30 - int ov[MAX_SUBSTRINGS]; - const char *str_ptr = NULL; - char *ptr = NULL; - uint32_t rc; - - ret = pcre_exec(parse_regex, parse_regex_study, rawstr, strlen(rawstr), 0, 0, ov, MAX_SUBSTRINGS); - if (ret < 1) { - SCLogError(SC_ERR_PCRE_MATCH, "pcre_exec parse error, ret %" PRId32 ", string %s", ret, rawstr); - return -1; - } - - res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 0, &str_ptr); - if (res < 0) { - SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed"); - return -1; - } - - ptr = (char *)str_ptr; - - if(ptr == NULL) - return -1; - - rc = (uint32_t )atol(ptr); - - SCFree(ptr); - return rc; } /** @@ -131,11 +67,41 @@ static uint32_t DetectGidParse (char *rawstr) */ static int DetectGidSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr) { - s->gid = DetectGidParse(rawstr); + char *str = rawstr; + char dubbed = 0; + + /* strip "'s */ + if (rawstr[0] == '\"' && rawstr[strlen(rawstr)-1] == '\"') { + str = SCStrdup(rawstr+1); + if (unlikely(str == NULL)) + return -1; + + str[strlen(rawstr)-2] = '\0'; + dubbed = 1; + } + + unsigned long gid = 0; + char *endptr = NULL; + gid = strtoul(rawstr, &endptr, 10); + if (endptr == NULL || *endptr != '\0') { + SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid character as arg " + "to gid keyword"); + goto error; + } + if (gid >= UINT_MAX) { + SCLogError(SC_ERR_INVALID_NUMERIC_VALUE, "gid value to high, max %u", UINT_MAX); + goto error; + } - if(s->gid > 0) - return 0; + s->gid = (uint32_t)gid; + if (dubbed) + SCFree(str); + return 0; + + error: + if (dubbed) + SCFree(str); return -1; } @@ -151,16 +117,22 @@ static int DetectGidSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr) * \retval 0 on failure */ static int GidTestParse01 (void) { - - int gid = 0; - - gid = DetectGidParse("1"); - - if (gid == 1) { - return 1; - } - - return 0; + int result = 0; + Signature *s = NULL; + + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + if (de_ctx == NULL) + goto end; + + s = DetectEngineAppendSig(de_ctx, "alert tcp 1.2.3.4 any -> any any (sid:1; gid:1;)"); + if (s == NULL || s->gid != 1) + goto end; + + result = 1; +end: + if (de_ctx != NULL) + DetectEngineCtxFree(de_ctx); + return result; } /** @@ -170,16 +142,20 @@ static int GidTestParse01 (void) { * \retval 0 on failure */ static int GidTestParse02 (void) { + int result = 0; - int gid = 0; + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + if (de_ctx == NULL) + goto end; - gid = DetectGidParse("a"); + if (DetectEngineAppendSig(de_ctx, "alert tcp 1.2.3.4 any -> any any (sid:1; gid:a;)") != NULL) + goto end; - if (gid > 1) { - return 1; - } - - return 0; + result = 1; +end: + if (de_ctx != NULL) + DetectEngineCtxFree(de_ctx); + return result; } #endif /* UNITTESTS */ @@ -189,6 +165,6 @@ static int GidTestParse02 (void) { void GidRegisterTests(void) { #ifdef UNITTESTS UtRegisterTest("GidTestParse01", GidTestParse01, 1); - UtRegisterTest("GidTestParse02", GidTestParse02, 0); + UtRegisterTest("GidTestParse02", GidTestParse02, 1); #endif /* UNITTESTS */ } diff --git a/src/detect-parse.c b/src/detect-parse.c index c3ba7bf1c4..804a7a74d1 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -2277,6 +2277,60 @@ end: return result; } +/** \test sid value too large. Bug #779 */ +static int SigParseTest18 (void) { + int result = 0; + + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + if (de_ctx == NULL) + goto end; + + if (DetectEngineAppendSig(de_ctx, "alert tcp 1.2.3.4 any -> !1.2.3.4 any (msg:\"SigParseTest01\"; sid:99999999999999999999;)") != NULL) + goto end; + + result = 1; +end: + if (de_ctx != NULL) + DetectEngineCtxFree(de_ctx); + return result; +} + +/** \test gid value too large. Related to bug #779 */ +static int SigParseTest19 (void) { + int result = 0; + + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + if (de_ctx == NULL) + goto end; + + if (DetectEngineAppendSig(de_ctx, "alert tcp 1.2.3.4 any -> !1.2.3.4 any (msg:\"SigParseTest01\"; sid:1; gid:99999999999999999999;)") != NULL) + goto end; + + result = 1; +end: + if (de_ctx != NULL) + DetectEngineCtxFree(de_ctx); + return result; +} + +/** \test rev value too large. Related to bug #779 */ +static int SigParseTest20 (void) { + int result = 0; + + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + if (de_ctx == NULL) + goto end; + + if (DetectEngineAppendSig(de_ctx, "alert tcp 1.2.3.4 any -> !1.2.3.4 any (msg:\"SigParseTest01\"; sid:1; rev:99999999999999999999;)") != NULL) + goto end; + + result = 1; +end: + if (de_ctx != NULL) + DetectEngineCtxFree(de_ctx); + return result; +} + /** \test Direction operator validation (invalid) */ int SigParseBidirecTest06 (void) { int result = 1; @@ -3156,6 +3210,9 @@ void SigParseRegisterTests(void) { UtRegisterTest("SigParseTest15", SigParseTest15, 1); UtRegisterTest("SigParseTest16", SigParseTest16, 1); UtRegisterTest("SigParseTest17", SigParseTest17, 1); + UtRegisterTest("SigParseTest18", SigParseTest18, 1); + UtRegisterTest("SigParseTest19", SigParseTest19, 1); + UtRegisterTest("SigParseTest20", SigParseTest20, 1); UtRegisterTest("SigParseBidirecTest06", SigParseBidirecTest06, 1); UtRegisterTest("SigParseBidirecTest07", SigParseBidirecTest07, 1); diff --git a/src/detect-rev.c b/src/detect-rev.c index 94e90e204a..5496200321 100644 --- a/src/detect-rev.c +++ b/src/detect-rev.c @@ -59,10 +59,15 @@ static int DetectRevSetup (DetectEngineCtx *de_ctx, Signature *s, char *rawstr) char *endptr = NULL; rev = strtoul(rawstr, &endptr, 10); if (endptr == NULL || *endptr != '\0') { - SCLogError(SC_ERR_INVALID_SIGNATURE, "Saw an invalid character as arg " + SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid character as arg " "to rev keyword"); goto error; } + if (rev >= UINT_MAX) { + SCLogError(SC_ERR_INVALID_NUMERIC_VALUE, "rev value to high, max %u", UINT_MAX); + goto error; + } + s->rev = (uint32_t)rev; if (dubbed) diff --git a/src/detect-sid.c b/src/detect-sid.c index 7d2f15eb04..452d83572a 100644 --- a/src/detect-sid.c +++ b/src/detect-sid.c @@ -59,10 +59,15 @@ static int DetectSidSetup (DetectEngineCtx *de_ctx, Signature *s, char *sidstr) char *endptr = NULL; id = strtoul(sidstr, &endptr, 10); if (endptr == NULL || *endptr != '\0') { - SCLogError(SC_ERR_INVALID_SIGNATURE, "Saw an invalid character as arg " + SCLogError(SC_ERR_INVALID_SIGNATURE, "invalid character as arg " "to sid keyword"); goto error; } + if (id >= UINT_MAX) { + SCLogError(SC_ERR_INVALID_NUMERIC_VALUE, "sid value to high, max %u", UINT_MAX); + goto error; + } + s->id = (uint32_t)id; if (dubbed)