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

diff --git a/src/detect-content.c b/src/detect-content.c
index fb33934685bf4c5ed8b4f7d6bce636850458977d..1fa14e102faa1218d8e341d3812444f7ee750dcf 100644 (file)
--- 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 d5f64ff0b4e8f340f831b4f4cb75ba6c4f9d20ee..f33f2e84cab872140c66fb6b91c0aaa25e716797 100644 (file)
--- 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 b645a3736305e6e8659f5b01acffaecf02adb61b..193bcc21e614fd5f3c593e81d6e7686f379baf66 100644 (file)
--- 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 c0385dbc6cbffe7cd7e2621acc823950ce9957f8..ab8541afa9ed7c6206ea35c64558a552fe574257 100644 (file)
--- 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 c7c60cf62674b4a822dec256079a40fe6892e6b1..03106912824c262e26540bc4fea9f220f14366b2 100644 (file)
--- 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 adbdab17ee68a0d1c7e6c5401e0db5f8ca288ebf..fdf2a11d277e217d8faa1e7a73c0e8b205e81d7e 100644 (file)
--- 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)
 {