From: Tom Peters <thopeter@cisco.com>
Date: Wed, 1 Oct 2014 20:24:13 +0000 (-0400)
Subject: header lines ending in LF without CR
X-Git-Tag: 3.0.0-233~1399
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dfc7c49a044ffaf156c0c2df4234312fd97e36c2;p=thirdparty%2Fsnort3.git

header lines ending in LF without CR
---

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.cc b/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.cc
index 5f4412db5..f8b693f6b 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.cc
@@ -52,10 +52,12 @@ void NHttpMsgHeadShared::analyze() {
 void NHttpMsgHeadShared::parse_header_block() {
     int32_t bytes_used = 0;
     num_headers = 0;
+    int num_seps;
     while (bytes_used < msg_text.length) {
         header_line[num_headers].start = msg_text.start + bytes_used;
-        header_line[num_headers].length = find_crlf(header_line[num_headers].start, msg_text.length - bytes_used);
-        bytes_used += header_line[num_headers++].length + 2;
+        header_line[num_headers].length = find_header_end(header_line[num_headers].start, msg_text.length - bytes_used,
+           &num_seps);
+        bytes_used += header_line[num_headers++].length + num_seps;
         if (num_headers >= MAXHEADERS) {
              break;
         }
@@ -65,6 +67,32 @@ void NHttpMsgHeadShared::parse_header_block() {
     }
 }
 
+// Return the number of octets before the CRLF that ends a header. CRLF does not count when immediately followed by
+// <SP> or <LF>. These whitespace characters at the beginning of the next line indicate that the previous header has
+// wrapped and is continuing on the next line.
+// 
+// The final header in the block will not be terminated by CRLF (splitter design) but will terminate at the end of the
+// buffer. length is returned.
+//
+// Bare LF without CR is accepted as the terminator unless preceded by backslash character. FIXIT-L this does not
+// consider whether \LF is contained within a quoted string and perhaps this should be revisited. The current
+// approach errs in the direction of not incorrectly dividing a single header into two headers.
+//
+// FIXIT-M any abuse of backslashes in headers should be a preprocessor alarm.
+
+uint32_t NHttpMsgHeadShared::find_header_end(const uint8_t* buffer, int32_t length, int* const num_seps) {
+    for (int32_t k=0; k < length-1; k++) {
+        if ((buffer[k] != '\\') && (buffer[k+1] == '\n')) {
+            if ((k+2 >= length) || ((buffer[k+2] != ' ') && (buffer[k+2] != '\t'))) {
+                *num_seps = (buffer[k] == '\r') ? 2 : 1;
+                return k + 2 - *num_seps;
+            }
+        }
+    }
+    *num_seps = 0;
+    return length;
+}
+
 // Divide header field lines into field name and field value
 void NHttpMsgHeadShared::parse_header_lines() {
     int colon;
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.h b/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.h
index 6e014e3a3..b49f32900 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_head_shared.h
@@ -73,6 +73,7 @@ protected:
     static const StrCode trans_code_list[];
 
     void parse_header_block();
+    static uint32_t find_header_end(const uint8_t* buffer, int32_t length, int* const num_seps);
     void parse_header_lines();
     void derive_header_name_id(int index);
 
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_request.cc b/src/service_inspectors/nhttp_inspect/nhttp_msg_request.cc
index 959bb5267..491a6af83 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_msg_request.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_request.cc
@@ -47,7 +47,6 @@ NHttpMsgRequest::NHttpMsgRequest(const uint8_t *buffer, const uint16_t buf_size,
    transaction->set_request(this);
 }
 
-
 void NHttpMsgRequest::parse_start_line() {
     // There should be exactly two spaces. One following the method and one before "HTTP/".
     // Additional spaces located within the URI are not allowed but we will tolerate it
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_section.cc b/src/service_inspectors/nhttp_inspect/nhttp_msg_section.cc
index fb4ffbc93..163536c44 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_msg_section.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_section.cc
@@ -56,17 +56,6 @@ NHttpMsgSection::NHttpMsgSection(const uint8_t *buffer, const uint16_t buf_size,
    delete_msg_on_destruct(buf_owner)
 {}
 
-// Return the number of octets before the CRLF that ends a header. Return length if CRLF not present. CRLF does not
-// count when immediately followed by <SP> or <LF>. These whitespace characters at the beginning of the next line
-// indicate that the previous header has wrapped and is continuing on the next line.
-uint32_t NHttpMsgSection::find_crlf(const uint8_t* buffer, int32_t length) {
-    for (int32_t k=0; k < length-1; k++) {
-        if ((buffer[k] == '\r') && (buffer[k+1] == '\n'))
-            if ((k+2 >= length) || ((buffer[k+2] != ' ') && (buffer[k+2] != '\t'))) return k;
-    }
-    return length;
-}
-
 void NHttpMsgSection::print_message_title(FILE *output, const char *title) const {
     fprintf(output, "HTTP message %s:\n", title);
     msg_text.print(output, "Input");
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_section.h b/src/service_inspectors/nhttp_inspect/nhttp_msg_section.h
index 31435a01f..f82c5c1fe 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_msg_section.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_section.h
@@ -59,7 +59,6 @@ protected:
        NHttpEnums::SourceId source_id_, bool buf_owner);
 
     // Convenience methods
-    static uint32_t find_crlf(const uint8_t* buffer, int32_t length);
     void print_message_title(FILE *output, const char *title) const;
     void print_message_wrapup(FILE *output) const;
     void create_event(NHttpEnums::EventSid sid);