CVE-2017-15377: Insufficient Information (NVD-CWE-noinfo)
value: In Suricata before 4.x, it was possible to trigger lots of redundant checks on the content of crafted network traffic with a certain signature, because of DetectEngineContentInspection in detect-engine-content-inspection.c. The search engine doesn't stop when it should after no match is found; instead, it stops only upon reaching inspection-recursion-limit (3000 by default).
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 7.5
Published: 2017-10-23T08:29Z
Repository: https://github.com/OISF/suricata
Language: C
Files changed: src/detect-engine-content-inspection.c
Commit date: 2017-05-01 11:59:03 +0200
Commit message: detect: avoid needless recursive scanning

Don't recursively inspect a detect list if the recursion
doesn't increase chance of success.

Diff: diff --git a/src/detect-engine-content-inspection.c b/src/detect-engine-content-inspection.c
index e7f83da7..71aa1e81 100644
--- a/src/detect-engine-content-inspection.c
+++ b/src/detect-engine-content-inspection.c
@@ -1,6 +1,6 @@
-/* Copyright (C) 2007-2011 Open Information Security Foundation
+/* Copyright (C) 2007-2017 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
  * Software Foundation.
  *
@@ -284,10 +284,15 @@ int DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCtx
             /* next we evaluate the result in combination with the
              * negation flag. */
             SCLogDebug("found %p cd negated %s", found, cd->flags & DETECT_CONTENT_NEGATED ? "true" : "false");
 
             if (found == NULL && !(cd->flags & DETECT_CONTENT_NEGATED)) {
+                if ((cd->flags & (DETECT_CONTENT_DISTANCE|DETECT_CONTENT_WITHIN)) == 0) {
+                    /* independent match from previous matches, so failure is fatal */
+                    det_ctx->discontinue_matching = 1;
+                }
+
                 goto no_match;
             } else if (found == NULL && (cd->flags & DETECT_CONTENT_NEGATED)) {
                 goto match;
             } else if (found != NULL && (cd->flags & DETECT_CONTENT_NEGATED)) {
                 SCLogDebug("content %"PRIu32" matched at offset %"PRIu32", but negated so no match", cd->id, match_offset);
@@ -308,20 +313,14 @@ int DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCtx
                         det_ctx->replist = DetectReplaceAddToList(det_ctx->replist, found, cd);
                     } else {
                         SCLogWarning(SC_ERR_INVALID_VALUE, "Can't modify payload without packet");
                     }
                 }
-                if (!(cd->flags & DETECT_CONTENT_RELATIVE_NEXT)) {
-                    SCLogDebug("no relative match coming up, so this is a match");
-                    goto match;
-                }
 
-                /* bail out if we have no next match. Technically this is an
-                 * error, as the current cd has the DETECT_CONTENT_RELATIVE_NEXT
-                 * flag set. */
+                /* if this is the last match we're done */
                 if (smd->is_last) {
-                    goto no_match;
+                    goto match;
                 }
 
                 SCLogDebug("content %"PRIu32, cd->id);
                 KEYWORD_PROFILING_END(det_ctx, smd->type, 1);
 
@@ -331,13 +330,25 @@ int DetectEngineContentInspection(DetectEngineCtx *de_ctx, DetectEngineThreadCtx
                 int r = DetectEngineContentInspection(de_ctx, det_ctx, s, smd+1,
                         f, buffer, buffer_len, stream_start_offset, inspection_mode, data);
                 if (r == 1) {
                     SCReturnInt(1);
                 }
+                SCLogDebug("no match for 'next sm'");
 
-                if (det_ctx->discontinue_matching)
+                if (det_ctx->discontinue_matching) {
+                    SCLogDebug("'next sm' said to discontinue this right now");
                     goto no_match;
+                }
+
+                /* no match and no reason to look for another instance */
+                if ((cd->flags & DETECT_CONTENT_RELATIVE_NEXT) == 0) {
+                    SCLogDebug("'next sm' does not depend on me, so we can give up");
+                    det_ctx->discontinue_matching = 1;
+                    goto no_match;
+                }
+
+                SCLogDebug("'next sm' depends on me %p, lets see what we can do (flags %u)", cd, cd->flags);
 
                 /* set the previous match offset to the start of this match + 1 */
                 prev_offset = (match_offset - (cd->content_len - 1));
                 SCLogDebug("trying to see if there is another match after prev_offset %"PRIu32, prev_offset);
             }

Vulnerable code: /* Copyright (C) 2007-2011 Open Information Security Foundation
                if (!(cd->flags & DETECT_CONTENT_RELATIVE_NEXT)) {
                    SCLogDebug("no relative match coming up, so this is a match");
                    goto match;
                }
                /* bail out if we have no next match. Technically this is an
                 * error, as the current cd has the DETECT_CONTENT_RELATIVE_NEXT
                 * flag set. */
                    goto no_match;
                if (det_ctx->discontinue_matching)
Fixed code: /* Copyright (C) 2007-2017 Open Information Security Foundation
                if ((cd->flags & (DETECT_CONTENT_DISTANCE|DETECT_CONTENT_WITHIN)) == 0) {
                    /* independent match from previous matches, so failure is fatal */
                    det_ctx->discontinue_matching = 1;
                }

                /* if this is the last match we're done */
                    goto match;
                SCLogDebug("no match for 'next sm'");
                if (det_ctx->discontinue_matching) {
                    SCLogDebug("'next sm' said to discontinue this right now");
                }

                /* no match and no reason to look for another instance */
                if ((cd->flags & DETECT_CONTENT_RELATIVE_NEXT) == 0) {
                    SCLogDebug("'next sm' does not depend on me, so we can give up");
                    det_ctx->discontinue_matching = 1;
                    goto no_match;
                }

                SCLogDebug("'next sm' depends on me %p, lets see what we can do (flags %u)", cd, cd->flags);