]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/content: fix FNs due to bad depth calc
authorVictor Julien <vjulien@oisf.net>
Thu, 9 Jun 2022 20:25:44 +0000 (22:25 +0200)
committerVictor Julien <vjulien@oisf.net>
Tue, 14 Jun 2022 08:14:29 +0000 (10:14 +0200)
When trying to propegate the depth/offset, within/distance chains
a logic error would set too a restrictive depth on a pattern that
followed more than one "unchained" patterns.

Bug: #5162.
(cherry picked from commit 8d20b40cdd3c8e911b0c4b06fb4fdc999b2d5c7c)

src/detect-content.c

index 05304b2449a52fca38fb97a368b567de462f28d8..4cde2abdc53cd547289aae3aa5f13140638effa6 100644 (file)
@@ -452,14 +452,12 @@ void DetectContentPropagateLimits(Signature *s)
         uint16_t offset = 0;
         uint16_t offset_plus_pat = 0;
         uint16_t depth = 0;
-        bool last_reset = false; // TODO really last reset 'depth'
+        bool has_active_depth_chain = false;
 
         bool has_depth = false;
         bool has_ends_with = false;
         uint16_t ends_with_depth = 0;
 
-        bool have_anchor = false;
-
         SigMatch *sm = s->init_data->smlists[list];
         for ( ; sm != NULL; sm = sm->next) {
             switch (sm->type) {
@@ -469,32 +467,30 @@ void DetectContentPropagateLimits(Signature *s)
                         offset = depth = 0;
                         offset_plus_pat = cd->content_len;
                         SCLogDebug("reset");
-                        last_reset = true;
-                        have_anchor = false;
+                        has_active_depth_chain = false;
                         continue;
                     }
                     if (cd->flags & DETECT_CONTENT_NEGATED) {
                         offset = depth = 0;
                         offset_plus_pat = 0;
                         SCLogDebug("reset because of negation");
-                        last_reset = true;
-                        have_anchor = false;
+                        has_active_depth_chain = false;
                         continue;
                     }
 
                     if (cd->depth) {
                         has_depth = true;
-                        have_anchor = true;
+                        has_active_depth_chain = true;
                     }
 
                     SCLogDebug("sm %p depth %u offset %u distance %d within %d", sm, cd->depth, cd->offset, cd->distance, cd->within);
-
                     SCLogDebug("stored: offset %u depth %u offset_plus_pat %u", offset, depth, offset_plus_pat);
 
-                    if ((cd->flags & DETECT_CONTENT_WITHIN) == 0) {
+                    if ((cd->flags & (DETECT_DEPTH | DETECT_CONTENT_WITHIN)) == 0) {
                         if (depth)
                             SCLogDebug("no within, reset depth");
                         depth = 0;
+                        has_active_depth_chain = false;
                     }
                     if ((cd->flags & DETECT_CONTENT_DISTANCE) == 0) {
                         if (offset_plus_pat)
@@ -502,51 +498,58 @@ void DetectContentPropagateLimits(Signature *s)
                         offset_plus_pat = offset = 0;
                     }
 
-                    SCLogDebug("stored: offset %u depth %u offset_plus_pat %u", offset, depth, offset_plus_pat);
-
+                    SCLogDebug("stored: offset %u depth %u offset_plus_pat %u "
+                               "has_active_depth_chain %s",
+                            offset, depth, offset_plus_pat,
+                            has_active_depth_chain ? "true" : "false");
                     if (cd->flags & DETECT_CONTENT_DISTANCE && cd->distance >= 0) {
                         VALIDATE((uint32_t)offset_plus_pat + cd->distance <= UINT16_MAX);
                         offset = cd->offset = (uint16_t)(offset_plus_pat + cd->distance);
                         SCLogDebug("updated content to have offset %u", cd->offset);
                     }
-                    if (have_anchor && !last_reset && offset_plus_pat && cd->flags & DETECT_CONTENT_WITHIN && cd->within >= 0) {
-                        if (depth && depth > offset_plus_pat) {
-                            int32_t dist = 0;
-                            if (cd->flags & DETECT_CONTENT_DISTANCE && cd->distance > 0) {
-                                dist = cd->distance;
-                                SCLogDebug("distance to add: %u. depth + dist %u", dist, depth + dist);
-                            }
-                            SCLogDebug("depth %u + cd->within %u", depth, cd->within);
-                            VALIDATE(depth + cd->within + dist >= 0 &&
-                                     depth + cd->within + dist <= UINT16_MAX);
-                            depth = cd->depth = (uint16_t)(depth + cd->within + dist);
-                        } else {
-                            SCLogDebug("offset %u + cd->within %u", offset, cd->within);
-                            VALIDATE(depth + cd->within >= 0 && depth + cd->within <= UINT16_MAX);
-                            depth = cd->depth = (uint16_t)(offset + cd->within);
-                        }
-                        SCLogDebug("updated content to have depth %u", cd->depth);
-                    } else {
-                        if (cd->depth == 0 && depth != 0) {
-                            if (cd->within > 0) {
-                                SCLogDebug("within %d distance %d", cd->within, cd->distance);
-                                if (cd->flags & DETECT_CONTENT_DISTANCE && cd->distance >= 0) {
-                                    VALIDATE(offset_plus_pat + cd->distance >= 0 &&
-                                             offset_plus_pat + cd->distance <= UINT16_MAX);
-                                    cd->offset = (uint16_t)(offset_plus_pat + cd->distance);
-                                    SCLogDebug("updated content to have offset %u", cd->offset);
+                    if (has_active_depth_chain) {
+                        if (offset_plus_pat && cd->flags & DETECT_CONTENT_WITHIN &&
+                                cd->within >= 0) {
+                            if (depth && depth > offset_plus_pat) {
+                                int32_t dist = 0;
+                                if (cd->flags & DETECT_CONTENT_DISTANCE && cd->distance > 0) {
+                                    dist = cd->distance;
+                                    SCLogDebug("distance to add: %u. depth + dist %u", dist,
+                                            depth + dist);
                                 }
-
+                                SCLogDebug("depth %u + cd->within %u", depth, cd->within);
+                                VALIDATE(depth + cd->within + dist >= 0 &&
+                                         depth + cd->within + dist <= UINT16_MAX);
+                                depth = cd->depth = (uint16_t)(depth + cd->within + dist);
+                            } else {
+                                SCLogDebug("offset %u + cd->within %u", offset, cd->within);
                                 VALIDATE(depth + cd->within >= 0 &&
                                          depth + cd->within <= UINT16_MAX);
-                                depth = cd->depth = (uint16_t)(cd->within + depth);
-                                SCLogDebug("updated content to have depth %u", cd->depth);
-
-                                if (cd->flags & DETECT_CONTENT_ENDS_WITH) {
-                                    has_ends_with = true;
-                                    if (ends_with_depth == 0)
-                                        ends_with_depth = depth;
-                                    ends_with_depth = MIN(ends_with_depth, depth);
+                                depth = cd->depth = (uint16_t)(offset + cd->within);
+                            }
+                            SCLogDebug("updated content to have depth %u", cd->depth);
+                        } else {
+                            if (cd->depth == 0 && depth != 0) {
+                                if (cd->within > 0) {
+                                    SCLogDebug("within %d distance %d", cd->within, cd->distance);
+                                    if (cd->flags & DETECT_CONTENT_DISTANCE && cd->distance >= 0) {
+                                        VALIDATE(offset_plus_pat + cd->distance >= 0 &&
+                                                 offset_plus_pat + cd->distance <= UINT16_MAX);
+                                        cd->offset = (uint16_t)(offset_plus_pat + cd->distance);
+                                        SCLogDebug("updated content to have offset %u", cd->offset);
+                                    }
+
+                                    VALIDATE(depth + cd->within >= 0 &&
+                                             depth + cd->within <= UINT16_MAX);
+                                    depth = cd->depth = (uint16_t)(cd->within + depth);
+                                    SCLogDebug("updated content to have depth %u", cd->depth);
+
+                                    if (cd->flags & DETECT_CONTENT_ENDS_WITH) {
+                                        has_ends_with = true;
+                                        if (ends_with_depth == 0)
+                                            ends_with_depth = depth;
+                                        ends_with_depth = MIN(ends_with_depth, depth);
+                                    }
                                 }
                             }
                         }
@@ -585,30 +588,27 @@ void DetectContentPropagateLimits(Signature *s)
                         }
                     }
                     if ((cd->flags & (DETECT_CONTENT_WITHIN|DETECT_CONTENT_DEPTH)) == 0) {
-                        last_reset = true;
+                        has_active_depth_chain = false;
                         depth = 0;
-                    } else {
-                        last_reset = false;
                     }
                     break;
                 }
                 case DETECT_PCRE: {
                     // relative could leave offset_plus_pat set.
-                    DetectPcreData *pd = (DetectPcreData *)sm->ctx;
+                    const DetectPcreData *pd = (const DetectPcreData *)sm->ctx;
                     if (pd->flags & DETECT_PCRE_RELATIVE) {
                         depth = 0;
-                        last_reset = true;
                     } else {
                         SCLogDebug("non-anchored PCRE not supported, reset offset_plus_pat & offset");
                         offset_plus_pat = offset = depth = 0;
-                        last_reset = true;
                     }
+                    has_active_depth_chain = false;
                     break;
                 }
                 default: {
                     SCLogDebug("keyword not supported, reset offset_plus_pat & offset");
                     offset_plus_pat = offset = depth = 0;
-                    last_reset = true;
+                    has_active_depth_chain = false;
                     break;
                 }
             }
@@ -732,6 +732,19 @@ static int DetectContentDepthTest01(void)
     // distance value is too high so we bail and not set anything on this content
     TEST_RUN("content:\"0123456789\"; content:\"abcdef\"; distance:2147483647;", 0, 0);
 
+    // Bug #5162.
+    TEST_RUN("content:\"SMB\"; depth:8; content:\"|09 00|\"; distance:8; within:2;", 11, 18);
+    TEST_RUN("content:\"SMB\"; depth:8; content:\"|09 00|\"; distance:8; within:2; content:\"|05 "
+             "00 00|\"; distance:0;",
+            13, 0);
+    TEST_RUN("content:\"SMB\"; depth:8; content:\"|09 00|\"; distance:8; within:2; content:\"|05 "
+             "00 00|\"; distance:0; content:\"|0c 00|\"; distance:19; within:2;",
+            35, 0);
+    TEST_RUN("content:\"SMB\"; depth:8; content:\"|09 00|\"; distance:8; within:2; content:\"|05 "
+             "00 00|\"; distance:0; content:\"|0c 00|\"; distance:19; within:2; content:\"|15 00 "
+             "00 00|\"; distance:20; within:4;",
+            57, 0);
+
     TEST_DONE;
 }