From 3536ba73483f748a60dca34242aef905995ea2b9 Mon Sep 17 00:00:00 2001 From: Anoop Saldanha Date: Sun, 25 Jul 2010 16:06:54 +0530 Subject: [PATCH] fix seg fault due to premature cleanup/double cleanup for byte(jump|test), isdataat, on seeing no previous relative keywords --- src/detect-bytejump.c | 12 +++--- src/detect-bytetest.c | 12 +++--- src/detect-engine-payload.c | 78 +++++++++++++++++++++++++++++++++++++ src/detect-isdataat.c | 12 +++--- 4 files changed, 96 insertions(+), 18 deletions(-) diff --git a/src/detect-bytejump.c b/src/detect-bytejump.c index 475bbdaaf5..2fe562ea73 100644 --- a/src/detect-bytejump.c +++ b/src/detect-bytejump.c @@ -588,11 +588,11 @@ int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (s->alproto == ALPROTO_DCERPC) { SCLogDebug("No preceding content or pcre keyword. Possible " "since this is an alproto sig."); - SCReturnInt(0); + return 0; } else { SCLogError(SC_ERR_INVALID_SIGNATURE, "No preceding content " "or uricontent or pcre option"); - goto error; + return -1; } } @@ -607,7 +607,7 @@ int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (cd == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } cd->flags |= DETECT_CONTENT_RELATIVE_NEXT; @@ -619,7 +619,7 @@ int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (ud == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } ud->flags |= DETECT_URICONTENT_RELATIVE_NEXT; @@ -630,7 +630,7 @@ int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (pe == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } pe->flags |= DETECT_PCRE_RELATIVE_NEXT; @@ -646,7 +646,7 @@ int DetectBytejumpSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) /* this will never hit */ SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - break; + return -1; } /* switch */ return 0; diff --git a/src/detect-bytetest.c b/src/detect-bytetest.c index d1e3df4cf1..0212845b22 100644 --- a/src/detect-bytetest.c +++ b/src/detect-bytetest.c @@ -604,11 +604,11 @@ int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (s->alproto == ALPROTO_DCERPC) { SCLogDebug("No preceding content or pcre keyword. Possible " "since this is an alproto sig."); - SCReturnInt(0); + return 0; } else { SCLogError(SC_ERR_INVALID_SIGNATURE, "No preceding content " "or uricontent or pcre option"); - goto error; + return -1; } } @@ -623,7 +623,7 @@ int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (cd == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } cd->flags |= DETECT_CONTENT_RELATIVE_NEXT; @@ -635,7 +635,7 @@ int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (ud == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } ud->flags |= DETECT_URICONTENT_RELATIVE_NEXT; @@ -646,7 +646,7 @@ int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) if (pe == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } pe->flags |= DETECT_PCRE_RELATIVE_NEXT; @@ -662,7 +662,7 @@ int DetectBytetestSetup(DetectEngineCtx *de_ctx, Signature *s, char *optstr) /* this will never hit */ SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - break; + return -1; } /* switch */ return 0; diff --git a/src/detect-engine-payload.c b/src/detect-engine-payload.c index 0b8e5d4ede..e22164ca66 100644 --- a/src/detect-engine-payload.c +++ b/src/detect-engine-payload.c @@ -617,6 +617,81 @@ end: return result; } +/** + * \test Test invalid sig. + */ +static int PayloadTestSig10(void) +{ + uint8_t *buf = (uint8_t *)"this is a super duper nova in super nova now"; + uint16_t buflen = strlen((char *)buf); + Packet *p = UTHBuildPacket( buf, buflen, IPPROTO_TCP); + int result = 0; + + char sig[] = "alert udp any any -> any any (msg:\"crash\"; " + "byte_test:4,>,2,0,relative; sid:11;)"; + + if (UTHPacketMatchSigMpm(p, sig, MPM_B2G) == 1) { + result = 0; + goto end; + } + + result = 1; +end: + if (p != NULL) + UTHFreePacket(p); + return result; +} + +/** + * \test Test invalid sig. + */ +static int PayloadTestSig11(void) +{ + uint8_t *buf = (uint8_t *)"this is a super duper nova in super nova now"; + uint16_t buflen = strlen((char *)buf); + Packet *p = UTHBuildPacket( buf, buflen, IPPROTO_TCP); + int result = 0; + + char sig[] = "alert udp any any -> any any (msg:\"crash\"; " + "byte_jump:1,0,relative; sid:11;)"; + + if (UTHPacketMatchSigMpm(p, sig, MPM_B2G) == 1) { + result = 0; + goto end; + } + + result = 1; +end: + if (p != NULL) + UTHFreePacket(p); + return result; +} + +/** + * \test Test invalid sig. + */ +static int PayloadTestSig12(void) +{ + uint8_t *buf = (uint8_t *)"this is a super duper nova in super nova now"; + uint16_t buflen = strlen((char *)buf); + Packet *p = UTHBuildPacket( buf, buflen, IPPROTO_TCP); + int result = 0; + + char sig[] = "alert udp any any -> any any (msg:\"crash\"; " + "isdataat:10,relative; sid:11;)"; + + if (UTHPacketMatchSigMpm(p, sig, MPM_B2G) == 1) { + result = 0; + goto end; + } + + result = 1; +end: + if (p != NULL) + UTHFreePacket(p); + return result; +} + #endif /* UNITTESTS */ void PayloadRegisterTests(void) { @@ -630,5 +705,8 @@ void PayloadRegisterTests(void) { UtRegisterTest("PayloadTestSig07", PayloadTestSig07, 1); UtRegisterTest("PayloadTestSig08", PayloadTestSig08, 1); UtRegisterTest("PayloadTestSig09", PayloadTestSig09, 1); + UtRegisterTest("PayloadTestSig10", PayloadTestSig10, 1); + UtRegisterTest("PayloadTestSig11", PayloadTestSig11, 1); + UtRegisterTest("PayloadTestSig12", PayloadTestSig12, 1); #endif /* UNITTESTS */ } diff --git a/src/detect-isdataat.c b/src/detect-isdataat.c index d1941d497a..6d93ed3f1f 100644 --- a/src/detect-isdataat.c +++ b/src/detect-isdataat.c @@ -286,11 +286,11 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, char *isdataatst if (s->alproto == ALPROTO_DCERPC) { SCLogDebug("No preceding content or pcre keyword. Possible " "since this is an alproto sig."); - SCReturnInt(0); + return 0; } else { SCLogError(SC_ERR_INVALID_SIGNATURE, "No preceding content " "or uricontent or pcre option"); - goto error; + return -1; } } @@ -305,7 +305,7 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, char *isdataatst if (cd == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } cd->flags |= DETECT_CONTENT_RELATIVE_NEXT; @@ -317,7 +317,7 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, char *isdataatst if (ud == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } ud->flags |= DETECT_URICONTENT_RELATIVE_NEXT; @@ -328,7 +328,7 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, char *isdataatst if (pe == NULL) { SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - goto error; + return -1; } pe->flags |= DETECT_PCRE_RELATIVE_NEXT; @@ -344,7 +344,7 @@ int DetectIsdataatSetup (DetectEngineCtx *de_ctx, Signature *s, char *isdataatst /* this will never hit */ SCLogError(SC_ERR_INVALID_SIGNATURE, "Unknown previous-" "previous keyword!"); - break; + return -1; } /* switch */ return 0;