From e087d938833bbf4665fc90ddb66c6913e570a895 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Wed, 19 Jul 2017 12:16:48 +0200 Subject: [PATCH] detect: reject dsize rules that can't match Rules can contain conflicting statements and lead to a unmatchable rule. 2 examples are rejected by this patch: 1. dsize < content 2. dsize < content@offset Bug #2187 --- src/detect-content.c | 40 ++++++++++++++++++ src/detect-content.h | 1 + src/detect-dsize.c | 97 ++++++++++++++++++++++++++++++++++++++++++++ src/detect-dsize.h | 4 ++ src/detect-parse.c | 40 ++++++++++++++++++ src/detect.c | 96 ------------------------------------------- 6 files changed, 182 insertions(+), 96 deletions(-) diff --git a/src/detect-content.c b/src/detect-content.c index fb33934685..1fa14e102f 100644 --- a/src/detect-content.c +++ b/src/detect-content.c @@ -47,6 +47,7 @@ #include "pkt-var.h" #include "host.h" #include "util-profiling.h" +#include "detect-dsize.h" static void DetectContentRegisterTests(void); @@ -365,6 +366,45 @@ void DetectContentFree(void *ptr) SCReturn; } +/** + * \retval 1 valid + * \retval 0 invalid + */ +_Bool DetectContentPMATCHValidateCallback(const Signature *s) +{ + if (!(s->flags & SIG_FLAG_DSIZE)) { + return TRUE; + } + + int max_right_edge_i = SigParseGetMaxDsize(s); + if (max_right_edge_i < 0) { + return TRUE; + } + + uint32_t max_right_edge = (uint32_t)max_right_edge_i; + + const SigMatch *sm = s->init_data->smlists[DETECT_SM_LIST_PMATCH]; + for ( ; sm != NULL; sm = sm->next) { + if (sm->type != DETECT_CONTENT) + continue; + const DetectContentData *cd = (const DetectContentData *)sm->ctx; + uint32_t right_edge = cd->content_len + cd->offset; + if (cd->content_len > max_right_edge) { + SCLogError(SC_ERR_INVALID_SIGNATURE, + "signature can't match as content length %u is bigger than dsize %u", + cd->content_len, max_right_edge); + return FALSE; + } + if (right_edge > max_right_edge) { + SCLogError(SC_ERR_INVALID_SIGNATURE, + "signature can't match as content length %u with offset %u (=%u) is bigger than dsize %u", + cd->content_len, cd->offset, right_edge, max_right_edge); + return FALSE; + } + } + return TRUE; +} + #ifdef UNITTESTS /* UNITTESTS */ /** * \brief Print list of DETECT_CONTENT SigMatch's allocated in a diff --git a/src/detect-content.h b/src/detect-content.h index d5f64ff0b4..f33f2e84ca 100644 --- a/src/detect-content.h +++ b/src/detect-content.h @@ -116,5 +116,6 @@ int DetectContentSetup(DetectEngineCtx *de_ctx, Signature *s, const char *conten void DetectContentPrint(DetectContentData *); void DetectContentFree(void *); +_Bool DetectContentPMATCHValidateCallback(const Signature *s); #endif /* __DETECT_CONTENT_H__ */ diff --git a/src/detect-dsize.c b/src/detect-dsize.c index b645a37363..193bcc21e6 100644 --- a/src/detect-dsize.c +++ b/src/detect-dsize.c @@ -32,6 +32,7 @@ #include "flow-var.h" +#include "detect-content.h" #include "detect-dsize.h" #include "util-unittest.h" @@ -373,6 +374,102 @@ static _Bool PrefilterDsizeIsPrefilterable(const Signature *s) return FALSE; } +/** \brief get max dsize "depth" + * \param s signature to get dsize value from + * \retval depth or negative value + */ +int SigParseGetMaxDsize(const Signature *s) +{ + if (s->flags & SIG_FLAG_DSIZE && s->init_data->dsize_sm != NULL) { + const DetectDsizeData *dd = (const DetectDsizeData *)s->init_data->dsize_sm->ctx; + + switch (dd->mode) { + case DETECTDSIZE_LT: + case DETECTDSIZE_EQ: + return dd->dsize; + case DETECTDSIZE_RA: + return dd->dsize2; + case DETECTDSIZE_GT: + default: + SCReturnInt(-2); + } + } + SCReturnInt(-1); +} + +/** \brief set prefilter dsize pair + * \param s signature to get dsize value from + */ +void SigParseSetDsizePair(Signature *s) +{ + if (s->flags & SIG_FLAG_DSIZE && s->init_data->dsize_sm != NULL) { + DetectDsizeData *dd = (DetectDsizeData *)s->init_data->dsize_sm->ctx; + + uint16_t low = 0; + uint16_t high = 65535; + + switch (dd->mode) { + case DETECTDSIZE_LT: + low = 0; + high = dd->dsize; + break; + case DETECTDSIZE_EQ: + low = dd->dsize; + high = dd->dsize; + break; + case DETECTDSIZE_RA: + low = dd->dsize; + high = dd->dsize2; + break; + case DETECTDSIZE_GT: + low = dd->dsize; + high = 65535; + break; + } + s->dsize_low = low; + s->dsize_high = high; + + SCLogDebug("low %u, high %u", low, high); + } +} + +/** + * \brief Apply dsize as depth to content matches in the rule + * \param s signature to get dsize value from + */ +void SigParseApplyDsizeToContent(Signature *s) +{ + SCEnter(); + + if (s->flags & SIG_FLAG_DSIZE) { + SigParseSetDsizePair(s); + + int dsize = SigParseGetMaxDsize(s); + if (dsize < 0) { + /* nothing to do */ + return; + } + + SigMatch *sm = s->init_data->smlists[DETECT_SM_LIST_PMATCH]; + for ( ; sm != NULL; sm = sm->next) { + if (sm->type != DETECT_CONTENT) { + continue; + } + + DetectContentData *cd = (DetectContentData *)sm->ctx; + if (cd == NULL) { + continue; + } + + if (cd->depth == 0 || cd->depth >= dsize) { + cd->depth = (uint16_t)dsize; + SCLogDebug("updated %u, content %u to have depth %u " + "because of dsize.", s->id, cd->id, cd->depth); + } + } + } +} + /* * ONLY TESTS BELOW THIS COMMENT */ diff --git a/src/detect-dsize.h b/src/detect-dsize.h index c0385dbc6c..ab8541afa9 100644 --- a/src/detect-dsize.h +++ b/src/detect-dsize.h @@ -38,5 +38,9 @@ typedef struct DetectDsizeData_ { /* prototypes */ void DetectDsizeRegister (void); +int SigParseGetMaxDsize(const Signature *s); +void SigParseSetDsizePair(Signature *s); +void SigParseApplyDsizeToContent(Signature *s); + #endif /* __DETECT_DSIZE_H__ */ diff --git a/src/detect-parse.c b/src/detect-parse.c index c7c60cf626..0310691282 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1440,6 +1440,11 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) SCEnter(); /* run buffer type validation callbacks if any */ + if (s->init_data->smlists[DETECT_SM_LIST_PMATCH]) { + if (DetectContentPMATCHValidateCallback(s) == FALSE) + SCReturnInt(0); + } + int x; for (x = 0; x < nlists; x++) { if (s->init_data->smlists[x]) { @@ -3668,6 +3673,36 @@ static int SigParseTestUnblanacedQuotes01(void) PASS; } +static int SigParseTestContentGtDsize01(void) +{ + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + FAIL_IF_NULL(de_ctx); + de_ctx->flags |= DE_QUIET; + + Signature *s = SigInit(de_ctx, + "alert http any any -> any any (" + "dsize:21; content:\"0123456789001234567890|00 00|\"; " + "sid:1; rev:1;)"); + FAIL_IF_NOT_NULL(s); + + PASS; +} + +static int SigParseTestContentGtDsize02(void) +{ + DetectEngineCtx *de_ctx = DetectEngineCtxInit(); + FAIL_IF_NULL(de_ctx); + de_ctx->flags |= DE_QUIET; + + Signature *s = SigInit(de_ctx, + "alert http any any -> any any (" + "dsize:21; content:\"0123456789|00 00|\"; offset:10; " + "sid:1; rev:1;)"); + FAIL_IF_NOT_NULL(s); + + PASS; +} + #endif /* UNITTESTS */ void SigParseRegisterTests(void) @@ -3724,5 +3759,10 @@ void SigParseRegisterTests(void) UtRegisterTest("SigParseTestAppLayerTLS03", SigParseTestAppLayerTLS03); UtRegisterTest("SigParseTestUnblanacedQuotes01", SigParseTestUnblanacedQuotes01); + + UtRegisterTest("SigParseTestContentGtDsize01", + SigParseTestContentGtDsize01); + UtRegisterTest("SigParseTestContentGtDsize02", + SigParseTestContentGtDsize02); #endif /* UNITTESTS */ } diff --git a/src/detect.c b/src/detect.c index adbdab17ee..fdf2a11d27 100644 --- a/src/detect.c +++ b/src/detect.c @@ -2214,102 +2214,6 @@ static void SigInitStandardMpmFactoryContexts(DetectEngineCtx *de_ctx) return; } -/** \brief get max dsize "depth" - * \param s signature to get dsize value from - * \retval depth or negative value - */ -static int SigParseGetMaxDsize(Signature *s) -{ - if (s->flags & SIG_FLAG_DSIZE && s->init_data->dsize_sm != NULL) { - DetectDsizeData *dd = (DetectDsizeData *)s->init_data->dsize_sm->ctx; - - switch (dd->mode) { - case DETECTDSIZE_LT: - case DETECTDSIZE_EQ: - return dd->dsize; - case DETECTDSIZE_RA: - return dd->dsize2; - case DETECTDSIZE_GT: - default: - SCReturnInt(-2); - } - } - SCReturnInt(-1); -} - -/** \brief set prefilter dsize pair - * \param s signature to get dsize value from - */ -static void SigParseSetDsizePair(Signature *s) -{ - if (s->flags & SIG_FLAG_DSIZE && s->init_data->dsize_sm != NULL) { - DetectDsizeData *dd = (DetectDsizeData *)s->init_data->dsize_sm->ctx; - - uint16_t low = 0; - uint16_t high = 65535; - - switch (dd->mode) { - case DETECTDSIZE_LT: - low = 0; - high = dd->dsize; - break; - case DETECTDSIZE_EQ: - low = dd->dsize; - high = dd->dsize; - break; - case DETECTDSIZE_RA: - low = dd->dsize; - high = dd->dsize2; - break; - case DETECTDSIZE_GT: - low = dd->dsize; - high = 65535; - break; - } - s->dsize_low = low; - s->dsize_high = high; - - SCLogDebug("low %u, high %u", low, high); - } -} - -/** - * \brief Apply dsize as depth to content matches in the rule - * \param s signature to get dsize value from - */ -static void SigParseApplyDsizeToContent(Signature *s) -{ - SCEnter(); - - if (s->flags & SIG_FLAG_DSIZE) { - SigParseSetDsizePair(s); - - int dsize = SigParseGetMaxDsize(s); - if (dsize < 0) { - /* nothing to do */ - return; - } - - SigMatch *sm = s->init_data->smlists[DETECT_SM_LIST_PMATCH]; - for ( ; sm != NULL; sm = sm->next) { - if (sm->type != DETECT_CONTENT) { - continue; - } - - DetectContentData *cd = (DetectContentData *)sm->ctx; - if (cd == NULL) { - continue; - } - - if (cd->depth == 0 || cd->depth >= dsize) { - cd->depth = (uint16_t)dsize; - SCLogDebug("updated %u, content %u to have depth %u " - "because of dsize.", s->id, cd->id, cd->depth); - } - } - } -} - /** \brief Pure-PCRE or bytetest rule */ static int RuleInspectsPayloadHasNoMpm(const Signature *s) {