Misc snort fixes.

- search for content fields taking into account length of last match
- handle absolute path to file file inclusion not using $RULE_PATH
- parse longer tokens (saw emerging-threats rule with enormous pcre)
- content offset is relative to start of frame, *not* previous content match
- show content modifiers 'rawbytes' and 'http_user_agent'

Change-Id: I0a4e0b857c8049380ed6aa47e4a3d3649e84d4ad
Reviewed-on: https://code.wireshark.org/review/22211
Petri-Dish: Martin Mathieson <martin.r.mathieson@googlemail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>

diff --git a/epan/dissectors/packet-snort.c b/epan/dissectors/packet-snort.c
index def0941a63c1624d84e17aa95d3235df4e4820e6..4c054582c7c91f683bfd2b2b91816abd5678ccdd 100644 (file)
--- a/epan/dissectors/packet-snort.c
+++ b/epan/dissectors/packet-snort.c
@@ -286,7 +286,6 @@ static gboolean content_compare_case_insensitive(const guint8* memory, const cha
     return TRUE;
 }
 
-
 /* Move through the bytes of the tvbuff, looking for a match against the
  * regexp from the given content.
  */
@@ -303,7 +302,8 @@ static gboolean look_for_pcre(content_t *content, tvbuff_t *tvb, guint start_off
         return FALSE;
     }
 
-    /* Copy remaining bytes into NULL-terminated string. */
+    /* Copy remaining bytes into NULL-terminated string. Unfortunately, this interface does't allow
+       us to find patterns that involve bytes with value 0.. */
     int length_remaining = tvb_captured_length_remaining(tvb, start_offset);
     gchar *string = (gchar*)g_malloc(length_remaining + 1);
     tvb_memcpy(tvb, (void*)string, start_offset, length_remaining);
@@ -747,8 +747,9 @@ static void snort_show_alert(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo
 
     /* Can only find start if we have the rule and know the protocol */
     guint content_start_match = 0;
+    guint payload_start = 0;
     if (rule) {
-        content_start_match = get_content_start_match(rule, tree);
+        payload_start = content_start_match = get_content_start_match(rule, tree);
     }
 
     /* Snort output arrived and was previously stored - so add to tree */
@@ -935,9 +936,10 @@ static void snort_show_alert(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo
                 /* Look up offset of match. N.B. would only expect to see on first content... */
                 guint offset_to_add = 0;
 
-                /* May need to add absolute offset into packet... */
+                /* May need to start looking from absolute offset into packet... */
                 if (rule->contents[n].offset_set) {
-                    offset_to_add = rule->contents[n].offset;
+                    content_start_match = payload_start + rule->contents[n].offset;
+                    offset_to_add = 0;
                 }
                 /* ... or a number of bytes beyond the previous content match */
                 else if (rule->contents[n].distance_set) {
@@ -964,6 +966,12 @@ static void snort_show_alert(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo
                                               rule->contents[n].str,
                                               content_text_template,
                                               rule->contents[n].str);
+
+            /* Next match position will be after this one */
+            if (match_found) {
+                content_start_match = content_last_match_end;
+            }
+
             if (!attempt_match) {
                 proto_item_append_text(ti, " (no match attempt made)");
             }
@@ -972,6 +980,9 @@ static void snort_show_alert(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo
             if (rule->contents[n].fastpattern) {
                 proto_item_append_text(ti, " (fast_pattern)");
             }
+            if (rule->contents[n].rawbytes) {
+                proto_item_append_text(ti, " (rawbytes)");
+            }
             if (rule->contents[n].nocase) {
                 proto_item_append_text(ti, " (nocase)");
             }
@@ -1001,6 +1012,9 @@ static void snort_show_alert(proto_tree *tree, tvbuff_t *tvb, packet_info *pinfo
             if (rule->contents[n].http_cookie != 0) {
                 proto_item_append_text(ti, " (http_cookie)");
             }
+            if (rule->contents[n].http_user_agent != 0) {
+                proto_item_append_text(ti, " (http_user_agent)");
+            }
 
             if (attempt_match && !rule->contents[n].negation && !match_found) {
                 /* Useful for debugging, may also happen when Snort is reassembling.. */

diff --git a/epan/dissectors/snort-config.c b/epan/dissectors/snort-config.c
index ccf412655974db54478db4f87cee0841c3cfe108..b8d17dcfe8fc8a76dfbd0d2e627ebb10dfb1bf12 100644 (file)
--- a/epan/dissectors/snort-config.c
+++ b/epan/dissectors/snort-config.c
@@ -73,7 +73,7 @@ static char *skipWhiteSpace(char *source, int *accumulated_offset)
  * - returns: requested string.  Returns from static buffer when copy is FALSE */
 static char* read_token(char* source, char delimeter, int *length, int *accumulated_length, gboolean copy)
 {
-    static char static_buffer[512];
+    static char static_buffer[1024];
     int offset = 0;
 
     char *source_proper = skipWhiteSpace(source, accumulated_length);
@@ -162,6 +162,14 @@ static void rule_set_content_fast_pattern(Rule_t *rule)
     }
 }
 
+/* Set the rawbytes property of a content field */
+static void rule_set_content_rawbytes(Rule_t *rule)
+{
+    if (rule->last_added_content) {
+        rule->last_added_content->rawbytes = TRUE;
+    }
+}
+
 /* Set the http_method property of a content field */
 static void rule_set_content_http_method(Rule_t *rule)
 {
@@ -186,6 +194,14 @@ static void rule_set_content_http_cookie(Rule_t *rule)
     }
 }
 
+/* Set the http_UserAgent property of a content field */
+static void rule_set_content_http_user_agent(Rule_t *rule)
+{
+    if (rule->last_added_content) {
+        rule->last_added_content->http_user_agent = TRUE;
+    }
+}
+
 /* Add a uricontent field to the rule */
 static gboolean rule_add_uricontent(Rule_t *rule, const char *uricontent_string, gboolean negated)
 {
@@ -537,12 +553,14 @@ static gboolean parse_include_file(SnortConfig_t *snort_config, char *line, cons
             /* Write rule path variable value */
             /* Don't assume $RULE_PATH will end in a file separator */
             if (snort_config->rule_path_is_absolute) {
+                /* Rule path is absolute, so it can go at start */
                 g_snprintf(substituted_filename, 512, "%s%s%s",
                            snort_config->rule_path,
                            g_file_separator,
                            include_filename + 10);
             }
             else {
+                /* Rule path is relative to config directory, so it goes first */
                 g_snprintf(substituted_filename, 512, "%s%s%s%s%s",
                            config_directory,
                            g_file_separator,
@@ -554,7 +572,13 @@ static gboolean parse_include_file(SnortConfig_t *snort_config, char *line, cons
         }
         else {
             /* No $RULE_PATH, just use directory and filename */
-            g_snprintf(substituted_filename, 512, "%s/%s", config_directory, include_filename);
+            /* But may not even need directory if included_folder is absolute! */
+            if (!g_path_is_absolute(include_filename)) {
+                g_snprintf(substituted_filename, 512, "%s/%s", config_directory, include_filename);
+            }
+            else {
+                g_strlcpy(substituted_filename, include_filename, 512);
+            }
         }
 
         /* Try to open the file. */
@@ -697,6 +721,12 @@ static void process_rule_option(Rule_t *rule, char *options, int option_start_of
     else if (strcmp(name, "http_cookie") == 0) {
         rule_set_content_http_cookie(rule);
     }
+    else if (strcmp(name, "http_user_agent") == 0) {
+        rule_set_content_http_user_agent(rule);
+    }
+    else if (strcmp(name, "rawbytes") == 0) {
+        rule_set_content_rawbytes(rule);
+    }
     else if (strcmp(name, "classtype") == 0) {
         rule_set_classtype(rule, value);
     }
@@ -1129,6 +1159,12 @@ gboolean content_convert_pcre_for_regex(content_t *content)
         return FALSE;
     }
 
+    if (pcre_length >= 512) {
+        /* Have seen regex library crash on very long expressions
+         * (830 bytes) as seen in SID=2019326, REV=6 */
+        return FALSE;
+    }
+
     /* Verify that string starts with / */
     if (content->str[0] != '/') {
         return FALSE;

diff --git a/epan/dissectors/snort-config.h b/epan/dissectors/snort-config.h
index 076e645aaea75714820581ff19e99d2201e47c7e..09a40ca1c9c2f9e4b92e1699ef6c50f71402ac48 100644 (file)
--- a/epan/dissectors/snort-config.h
+++ b/epan/dissectors/snort-config.h
@@ -57,10 +57,13 @@ typedef struct content_t {
 
     gboolean fastpattern; /* Is most distinctive content in rule */
 
+    gboolean rawbytes;    /* Match should be done against raw bytes (which we do anyway) */
+
     /* http preprocessor modifiers */
     gboolean http_method;
     gboolean http_client_body;
     gboolean http_cookie;
+    gboolean http_user_agent;
 
     /* Pattern converted into bytes for matching against packet.
        Used for regular patterns and PCREs alike. */