detect/pcre: fix memory read error in detect

Fix case where a HTTP modifier in PCRE statements in a rule that would not
set the http protocol, would lead to a HTTP condition being run against
a non-HTTP flow. This would lead to invalid memory access.

Fix by properly setting the alproto and SIG_FLAG_APPLAYER flag in the
signature, leading to the signature implicitly setting the protocol
so rejecting it for inspection when the flow has a different protocol.

Bug #2863

diff --git a/src/detect-pcre.c b/src/detect-pcre.c
index 20776ddd1ed0d3161e33ef17303b0482a3736bf8..515263b13d02c1256a9cf7ec49b4a7b2224d3eab 100644 (file)
--- a/src/detect-pcre.c
+++ b/src/detect-pcre.c
@@ -324,8 +324,9 @@ static int DetectPcreHasUpperCase(const char *re)
     return 0;
 }
 
-static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *regexstr, int *sm_list,
-        char *capture_names, size_t capture_names_size, bool negate)
+static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx,
+        const char *regexstr, int *sm_list, char *capture_names,
+        size_t capture_names_size, bool negate, AppProto *alproto)
 {
     int ec;
     const char *eb;
@@ -466,6 +467,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_uri");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'V': {
@@ -475,6 +477,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_user_agent");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'W': {
@@ -484,6 +487,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_host");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     check_host_header = 1;
                     break;
                 }
@@ -494,6 +498,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_raw_host");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'H': { /* snort's option */
@@ -503,6 +508,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_header");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 } case 'I': { /* snort's option */
                     if (pd->flags & DETECT_PCRE_RAWBYTES) {
@@ -511,11 +517,13 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_raw_uri");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'D': { /* snort's option */
                     int list = DetectBufferTypeGetByName("http_raw_header");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'M': { /* snort's option */
@@ -525,6 +533,7 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_method");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'C': { /* snort's option */
@@ -534,30 +543,35 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
                     }
                     int list = DetectBufferTypeGetByName("http_cookie");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'P': {
                     /* snort's option (http request body inspection) */
                     int list = DetectBufferTypeGetByName("http_client_body");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'Q': {
                     int list = DetectBufferTypeGetByName("file_data");
                     /* suricata extension (http response body inspection) */
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'Y': {
                     /* snort's option */
                     int list = DetectBufferTypeGetByName("http_stat_msg");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 case 'S': {
                     /* snort's option */
                     int list = DetectBufferTypeGetByName("http_stat_code");
                     *sm_list = DetectPcreSetList(*sm_list, list);
+                    *alproto = ALPROTO_HTTP;
                     break;
                 }
                 default:
@@ -661,7 +675,6 @@ static DetectPcreData *DetectPcreParse (DetectEngineCtx *de_ctx, const char *reg
     } else {
         goto error;
     }
-
     return pd;
 
 error:
@@ -817,9 +830,11 @@ static int DetectPcreSetup (DetectEngineCtx *de_ctx, Signature *s, const char *r
     int ret = -1;
     int parsed_sm_list = DETECT_SM_LIST_NOTSET;
     char capture_names[1024] = "";
+    AppProto alproto = ALPROTO_UNKNOWN;
 
     pd = DetectPcreParse(de_ctx, regexstr, &parsed_sm_list,
-            capture_names, sizeof(capture_names), s->init_data->negated);
+            capture_names, sizeof(capture_names), s->init_data->negated,
+            &alproto);
     if (pd == NULL)
         goto error;
     if (DetectPcreParseCapture(regexstr, de_ctx, pd, capture_names) < 0)
@@ -834,9 +849,19 @@ static int DetectPcreSetup (DetectEngineCtx *de_ctx, Signature *s, const char *r
             case DETECT_SM_LIST_NOTSET:
                 sm_list = DETECT_SM_LIST_PMATCH;
                 break;
-            default:
+            default: {
+                if (alproto != ALPROTO_UNKNOWN) {
+                    /* see if the proto doesn't conflict
+                     * with what we already have. */
+                    if (s->alproto != ALPROTO_UNKNOWN &&
+                            alproto != s->alproto) {
+                        goto error;
+                    }
+                    DetectSignatureSetAppProto(s, alproto);
+                }
                 sm_list = parsed_sm_list;
                 break;
+            }
         }
     }
     if (sm_list == -1)
@@ -918,8 +943,9 @@ static int DetectPcreParseTest01 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NOT_NULL(pd);
 
     DetectEngineCtxFree(de_ctx);
@@ -937,9 +963,11 @@ static int DetectPcreParseTest02 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NOT_NULL(pd);
+    FAIL_IF_NOT(alproto == ALPROTO_HTTP);
 
     DetectEngineCtxFree(de_ctx);
     return result;
@@ -956,8 +984,9 @@ static int DetectPcreParseTest03 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NOT_NULL(pd);
 
     DetectEngineCtxFree(de_ctx);
@@ -975,9 +1004,11 @@ static int DetectPcreParseTest04 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NULL(pd);
+    FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
 
     DetectPcreFree(pd);
     DetectEngineCtxFree(de_ctx);
@@ -995,9 +1026,11 @@ static int DetectPcreParseTest05 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NULL(pd);
+    FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
 
     DetectPcreFree(pd);
     DetectEngineCtxFree(de_ctx);
@@ -1015,9 +1048,11 @@ static int DetectPcreParseTest06 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NULL(pd);
+    FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
 
     DetectPcreFree(pd);
     DetectEngineCtxFree(de_ctx);
@@ -1035,9 +1070,11 @@ static int DetectPcreParseTest07 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NULL(pd);
+    FAIL_IF_NOT(alproto == ALPROTO_HTTP);
 
     DetectPcreFree(pd);
     DetectEngineCtxFree(de_ctx);
@@ -1055,9 +1092,11 @@ static int DetectPcreParseTest08 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NULL(pd);
+    FAIL_IF_NOT(alproto == ALPROTO_UNKNOWN);
 
     DetectPcreFree(pd);
     DetectEngineCtxFree(de_ctx);
@@ -1075,8 +1114,9 @@ static int DetectPcreParseTest09 (void)
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
     FAIL_IF_NULL(de_ctx);
+    AppProto alproto = ALPROTO_UNKNOWN;
 
-    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, teststring, &list, NULL, 0, false, &alproto);
     FAIL_IF_NULL(pd);
 
     DetectPcreFree(pd);
@@ -3419,30 +3459,30 @@ static int DetectPcreFlowvarCapture03(void)
  */
 static int DetectPcreParseHttpHost(void)
 {
-    DetectPcreData *pd = NULL;
+    AppProto alproto = ALPROTO_UNKNOWN;
     int list = DETECT_SM_LIST_NOTSET;
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
 
     FAIL_IF(de_ctx == NULL);
 
-    pd = DetectPcreParse(de_ctx, "/domain\\.com/W", &list, NULL, 0, false);
+    DetectPcreData *pd = DetectPcreParse(de_ctx, "/domain\\.com/W", &list, NULL, 0, false, &alproto);
     FAIL_IF(pd == NULL);
     DetectPcreFree(pd);
 
     list = DETECT_SM_LIST_NOTSET;
-    pd = DetectPcreParse(de_ctx, "/dOmain\\.com/W", &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, "/dOmain\\.com/W", &list, NULL, 0, false, &alproto);
     FAIL_IF(pd != NULL);
 
     /* Uppercase meta characters are valid. */
     list = DETECT_SM_LIST_NOTSET;
-    pd = DetectPcreParse(de_ctx, "/domain\\D+\\.com/W", &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, "/domain\\D+\\.com/W", &list, NULL, 0, false, &alproto);
     FAIL_IF(pd == NULL);
     DetectPcreFree(pd);
 
     /* This should not parse as the first \ escapes the second \, then
      * we have a D. */
     list = DETECT_SM_LIST_NOTSET;
-    pd = DetectPcreParse(de_ctx, "/\\\\Ddomain\\.com/W", &list, NULL, 0, false);
+    pd = DetectPcreParse(de_ctx, "/\\\\Ddomain\\.com/W", &list, NULL, 0, false, &alproto);
     FAIL_IF(pd != NULL);
 
     DetectEngineCtxFree(de_ctx);