]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect: fix legacy modifiers leading to multi-buffer
authorVictor Julien <vjulien@oisf.net>
Tue, 10 Oct 2023 10:09:09 +0000 (12:09 +0200)
committerVictor Julien <victor@inliniac.net>
Mon, 16 Oct 2023 19:16:37 +0000 (21:16 +0200)
Fix non-continious matches with content and pcre modifiers setting up
multiple buffers.

To address this store whether a buffer is multi-capable and if not reuse
an earlier buffer if possible.

Bug: #6397.

Fixes: ad88efc2d868 ("detect: support multi buffer matching")
src/detect-engine.c
src/detect-parse.c
src/detect.h

index 009741c503fbe1b8c0694d067dd18bf83d507fe5..d8f9f1880e56494088198c7f325d3a1a90727686 100644 (file)
@@ -1444,6 +1444,8 @@ int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int l
     s->init_data->curbuf->id = list;
     s->init_data->curbuf->head = NULL;
     s->init_data->curbuf->tail = NULL;
+    s->init_data->curbuf->multi_capable =
+            DetectEngineBufferTypeSupportsMultiInstanceGetById(de_ctx, list);
     SCLogDebug("new: idx %u list %d set up curbuf %p", s->init_data->buffer_index - 1, list,
             s->init_data->curbuf);
 
@@ -1470,6 +1472,7 @@ int DetectBufferGetActiveList(DetectEngineCtx *de_ctx, Signature *s)
         if (new_list == -1) {
             SCReturnInt(-1);
         }
+        int base_list = s->init_data->list;
         SCLogDebug("new_list %d", new_list);
         s->init_data->list = new_list;
         s->init_data->list_set = false;
@@ -1482,6 +1485,8 @@ int DetectBufferGetActiveList(DetectEngineCtx *de_ctx, Signature *s)
                 return -1;
             }
             s->init_data->curbuf = &s->init_data->buffers[s->init_data->buffer_index++];
+            s->init_data->curbuf->multi_capable =
+                    DetectEngineBufferTypeSupportsMultiInstanceGetById(de_ctx, base_list);
         }
         if (s->init_data->curbuf == NULL) {
             SCLogError("failed to setup buffer");
index 2e798d7b1cbf9556b62bbb67588cfdb8b5b2e7b1..b696b2055cb285a32420fa8fd80c020faa880da9 100644 (file)
@@ -289,19 +289,32 @@ int DetectEngineContentModifierBufferSetup(DetectEngineCtx *de_ctx,
             SCLogError("no matches for previous buffer");
             return -1;
         }
-        if (SignatureInitDataBufferCheckExpand(s) < 0) {
-            SCLogError("failed to expand rule buffer array");
-            return -1;
+        bool reuse_buffer = false;
+        if (s->init_data->curbuf != NULL && (int)s->init_data->curbuf->id != sm_list) {
+            for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
+                if (s->init_data->buffers[x].id == (uint32_t)sm_list) {
+                    s->init_data->curbuf = &s->init_data->buffers[x];
+                    reuse_buffer = true;
+                    break;
+                }
+            }
         }
 
-        /* initialize a new buffer */
-        s->init_data->curbuf = &s->init_data->buffers[s->init_data->buffer_index++];
-        s->init_data->curbuf->id = sm_list;
-        s->init_data->curbuf->head = NULL;
-        s->init_data->curbuf->tail = NULL;
-        SCLogDebug("idx %u list %d set up curbuf %p s->init_data->buffer_index %u",
-                s->init_data->buffer_index - 1, sm_list, s->init_data->curbuf,
-                s->init_data->buffer_index);
+        if (!reuse_buffer) {
+            if (SignatureInitDataBufferCheckExpand(s) < 0) {
+                SCLogError("failed to expand rule buffer array");
+                return -1;
+            }
+
+            /* initialize a new buffer */
+            s->init_data->curbuf = &s->init_data->buffers[s->init_data->buffer_index++];
+            s->init_data->curbuf->id = sm_list;
+            s->init_data->curbuf->head = NULL;
+            s->init_data->curbuf->tail = NULL;
+            SCLogDebug("idx %u list %d set up curbuf %p s->init_data->buffer_index %u",
+                    s->init_data->buffer_index - 1, sm_list, s->init_data->curbuf,
+                    s->init_data->buffer_index);
+        }
     }
 
     /* transfer the sm from the pmatch list to sm_list */
@@ -469,6 +482,18 @@ void SigMatchAppendSMToList(Signature *s, SigMatch *new, const int list)
             SCLogDebug("reset: list %d != s->init_data->list %d", list, s->init_data->list);
             s->init_data->list = DETECT_SM_LIST_NOTSET;
         }
+
+        if (s->init_data->curbuf != NULL && (int)s->init_data->curbuf->id != list) {
+            for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
+                if (s->init_data->buffers[x].id == (uint32_t)list &&
+                        !s->init_data->buffers[x].multi_capable) {
+                    SCLogDebug("reusing buffer %u as it isn't multi-capable", x);
+                    s->init_data->curbuf = &s->init_data->buffers[x];
+                    break;
+                }
+            }
+        }
+
         if ((s->init_data->curbuf != NULL && (int)s->init_data->curbuf->id != list) ||
                 s->init_data->curbuf == NULL) {
             if (SignatureInitDataBufferCheckExpand(s) < 0) {
index 69a5524e583c6bba3ce1e6a9a6e4434b799bc6f6..04dd49a65a755003f3909bd4530e2ae877431640 100644 (file)
@@ -515,6 +515,8 @@ typedef struct SignatureInitDataBuffer_ {
     bool sm_init; /**< initialized by sigmatch, which is likely something like `urilen:10; http.uri;
                      content:"abc";`. These need to be in the same list. Unset once `http.uri` is
                      set up. */
+    bool multi_capable; /**< true if we can have multiple instances of this buffer, so e.g. for
+                           http.uri. */
     /* sig match list */
     SigMatch *head;
     SigMatch *tail;