Merge pull request #966 in SNORT/snort3 from nhttp82 to master

Squashed commit of the following:

commit 4c13fff1d7016433321abccecaa42fc9900492b5
Author: Tom Peters <thopeter@cisco.com>
Date:   Thu Jul 13 16:08:57 2017 -0400

    http_inspect: added 119:97 for lower case letters in version field

diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index 330ef762e272edb4958d42687a8530c911cb3135..aa0e5480a832b40b2f1cedd3b1b8bd169187c089 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -69,7 +69,7 @@ ScanResult HttpStartCutter::cut(const uint8_t* buffer, uint32_t length,
             // The purpose of validate() is to quickly and efficiently dispose of obviously wrong
             // bindings. Passing is no guarantee that the connection is really HTTP, but failing
             // makes it clear that it isn't.
-            switch (validate(buffer[k]))
+            switch (validate(buffer[k], infractions, events))
             {
             case V_GOOD:
                 validated = true;
@@ -110,7 +110,8 @@ ScanResult HttpStartCutter::cut(const uint8_t* buffer, uint32_t length,
     return SCAN_NOTFOUND;
 }
 
-HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet)
+HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet, HttpInfractions*,
+    HttpEventGen*)
 {
     // Request line must begin with a method. There is no list of all possible methods because
     // extension is allowed, so there is no absolute way to tell whether something is a method.
@@ -128,15 +129,26 @@ HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet)
     return V_TBD;
 }
 
-HttpStartCutter::ValidationResult HttpStatusCutter::validate(uint8_t octet)
+HttpStartCutter::ValidationResult HttpStatusCutter::validate(uint8_t octet,
+    HttpInfractions* infractions, HttpEventGen* events)
 {
     // Status line must begin "HTTP/"
     static const int match_size = 5;
-    static const uint8_t match[match_size] = { 'H', 'T', 'T', 'P', '/' };
+    static const uint8_t primary_match[match_size] = { 'H', 'T', 'T', 'P', '/' };
+    static const uint8_t secondary_match[match_size] = { 'h', 't', 't', 'p', '/' };
 
-    if (octet != match[octets_checked++])
-        return V_BAD;
-    if (octets_checked >= match_size)
+    if (octet != primary_match[octets_checked])
+    {
+        if (octet == secondary_match[octets_checked])
+        {
+            // Lower case is wrong but we can still parse the message
+            *infractions += INF_VERSION_NOT_UPPERCASE;
+            events->create_event(EVENT_VERSION_NOT_UPPERCASE);
+        }
+        else
+            return V_BAD;
+    }
+    if (++octets_checked >= match_size)
         return V_GOOD;
     return V_TBD;
 }

diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h
index 58c8e453441c46bfe186fce441fd654d2ccee07c..cc4c36426e8c3f1b44d11fa9ff2fbc03a994dc8c 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.h
+++ b/src/service_inspectors/http_inspect/http_cutter.h
@@ -62,7 +62,7 @@ protected:
 
 private:
     static const int MAX_LEADING_WHITESPACE = 20;
-    virtual ValidationResult validate(uint8_t octet) = 0;
+    virtual ValidationResult validate(uint8_t octet, HttpInfractions*, HttpEventGen*) = 0;
     bool validated = false;
 };
 
@@ -70,14 +70,14 @@ class HttpRequestCutter : public HttpStartCutter
 {
 private:
     uint32_t octets_checked = 0;
-    ValidationResult validate(uint8_t octet) override;
+    ValidationResult validate(uint8_t octet, HttpInfractions*, HttpEventGen*) override;
 };
 
 class HttpStatusCutter : public HttpStartCutter
 {
 private:
     uint32_t octets_checked = 0;
-    ValidationResult validate(uint8_t octet) override;
+    ValidationResult validate(uint8_t octet, HttpInfractions*, HttpEventGen*) override;
 };
 
 class HttpHeaderCutter : public HttpCutter

diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index 31cd6bbdcd9ed833aec92a2c3f6e869a3a11d0a5..8055a97761eadd789ebd5152d66873b175db7fb4 100644 (file)
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -230,6 +230,7 @@ enum Infraction
     INF_REPEATED_HEADER,
     INF_CONTENT_ENCODING_CHUNKED,
     INF_206_WITHOUT_RANGE,
+    INF_VERSION_NOT_UPPERCASE,
     INF__MAX_VALUE
 };
 
@@ -340,6 +341,7 @@ enum EventSid
     EVENT_REPEATED_HEADER,
     EVENT_CONTENT_ENCODING_CHUNKED,
     EVENT_206_WITHOUT_RANGE,
+    EVENT_VERSION_NOT_UPPERCASE,
     EVENT__MAX_VALUE
 };
 

diff --git a/src/service_inspectors/http_inspect/http_msg_request.cc b/src/service_inspectors/http_inspect/http_msg_request.cc
index 8c5d3f576650b377ab620e7c6d4ed532441afe00..93221a6626a939c1ddc21cfca1fc222355f19447 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_request.cc
+++ b/src/service_inspectors/http_inspect/http_msg_request.cc
@@ -37,20 +37,35 @@ HttpMsgRequest::HttpMsgRequest(const uint8_t* buffer, const uint16_t buf_size,
 
 void HttpMsgRequest::parse_start_line()
 {
-    // Check the version field
+    // Version field
     if ((start_line.length() < 10) || !is_sp_tab[start_line.start()[start_line.length()-9]] ||
-         memcmp(start_line.start() + start_line.length() - 8, "HTTP/", 5))
+        memcmp(start_line.start() + start_line.length() - 8, "HTTP/", 5))
     {
-        if (!handle_zero_nine())
+        // Something is wrong with this message. Check for lower case letters in HTTP-name.
+        if ((start_line.length() >= 10) && is_sp_tab[start_line.start()[start_line.length()-9]] &&
+            http_name_nocase_ok(start_line.start() + start_line.length() - 8))
+        {
+            add_infraction(INF_VERSION_NOT_UPPERCASE);
+            create_event(EVENT_VERSION_NOT_UPPERCASE);
+        }
+        // Check for version 0.9 request.
+        else if (handle_zero_nine())
+        {
+            return;
+        }
+        // Just a plain old bad request
+        else
         {
-            // Just a plain old bad request
             add_infraction(INF_BAD_REQ_LINE);
             transaction->get_events(source_id)->generate_misformatted_http(start_line.start(),
                 start_line.length());
+            return;
         }
-        return;
     }
 
+    version.set(8, start_line.start() + (start_line.length() - 8));
+    derive_version_id();
+
     HttpModule::increment_peg_counts(PEG_REQUEST);
 
     // The splitter guarantees there will be a non-whitespace at octet 1 and a whitespace within
@@ -84,9 +99,6 @@ void HttpMsgRequest::parse_start_line()
     default: HttpModule::increment_peg_counts(PEG_OTHER_METHOD); break;
     }
 
-    version.set(8, start_line.start() + (start_line.length() - 8));
-    derive_version_id();
-
     if (first_end < last_begin)
     {
         uri = new HttpUri(start_line.start() + first_end + 1, last_begin - first_end - 1,
@@ -100,6 +112,15 @@ void HttpMsgRequest::parse_start_line()
     }
 }
 
+bool HttpMsgRequest::http_name_nocase_ok(const uint8_t* start)
+{
+    return ((start[0] == 'H') || (start[0] == 'h')) &&
+           ((start[1] == 'T') || (start[1] == 't')) &&
+           ((start[2] == 'T') || (start[2] == 't')) &&
+           ((start[3] == 'P') || (start[3] == 'p')) &&
+           (start[4] == '/');
+}
+
 bool HttpMsgRequest::handle_zero_nine()
 {
     // 0.9 request line is supposed to be "GET <URI>\r\n"

diff --git a/src/service_inspectors/http_inspect/http_msg_request.h b/src/service_inspectors/http_inspect/http_msg_request.h
index 82df4253f307e634f2656ec7433faf9d681a0ba4..cd23764ba26a195540cbb3577a0403d35a695aa6 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_request.h
+++ b/src/service_inspectors/http_inspect/http_msg_request.h
@@ -63,6 +63,7 @@ private:
     static const StrCode method_list[];
 
     void parse_start_line() override;
+    bool http_name_nocase_ok(const uint8_t* start);
     bool handle_zero_nine();
 
     Field method;

diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc
index ab1f2ec75bbe60be17761489bbd0e635375aeaad..8afed9a6f6d55423940512eed2b435e7a3d4da2e 100644 (file)
--- a/src/service_inspectors/http_inspect/http_tables.cc
+++ b/src/service_inspectors/http_inspect/http_tables.cc
@@ -377,6 +377,7 @@ const RuleMap HttpModule::http_events[] =
                                         "values" },
     { EVENT_CONTENT_ENCODING_CHUNKED,   "invalid value chunked in Content-Encoding header" },
     { EVENT_206_WITHOUT_RANGE,          "206 response sent to a request without a Range header" },
+    { EVENT_VERSION_NOT_UPPERCASE,      "'HTTP' in version field not all upper case" },
     { 0, nullptr }
 };