Merge pull request #1123 in SNORT/snort3 from nhttp95 to master

Squashed commit of the following:

commit 83b52927abd49f59d40d54966f789960fb7b4ec1
Author: Tom Peters <thopeter@cisco.com>
Date:   Sat Mar 3 13:08:08 2018 -0500

    http_inspect: white space before chunk length

diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index d8c81730e9c20c9639360f4d3286ee82b375ba4d..85c49eddf87b21597306c5e91ec19a61f0d6bbae 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -327,9 +327,26 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 events->create_event(EVENT_CHUNK_BAD_SEP);
                 break;
             }
-            curr_state = CHUNK_ZEROS;
+            curr_state = CHUNK_LEADING_WS;
             k--; // Reprocess this octet in the next state
             break;
+        case CHUNK_LEADING_WS:
+            // Looking for whitespace before the chunk size
+            if (is_sp_tab[buffer[k]])
+            {
+                *infractions += INF_CHUNK_LEADING_WS;
+                events->create_event(EVENT_CHUNK_WHITESPACE);
+                num_leading_ws++;
+                if (num_leading_ws == 5)
+                {
+                    events->create_event(EVENT_BROKEN_CHUNK);
+                    curr_state = CHUNK_BAD;
+                }
+                break;
+            }
+            curr_state = CHUNK_ZEROS;
+            k--;
+            break;
         case CHUNK_ZEROS:
             // Looking for leading zeros in the chunk size.
             if (buffer[k] == '0')
@@ -362,7 +379,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
             {
                 *infractions += INF_CHUNK_WHITESPACE;
                 events->create_event(EVENT_CHUNK_WHITESPACE);
-                curr_state = CHUNK_WHITESPACE;
+                curr_state = CHUNK_TRAILING_WS;
             }
             else if (buffer[k] == ';')
             {
@@ -389,7 +406,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 }
             }
             break;
-        case CHUNK_WHITESPACE:
+        case CHUNK_TRAILING_WS:
             // Skipping over improper whitespace following the chunk size
             if (buffer[k] == '\r')
             {
@@ -507,6 +524,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
         case CHUNK_DCRLF2:
             // The LF from the end-of-chunk CRLF should be here
             num_good_chunks++;
+            num_leading_ws = 0;
             num_zeros = 0;
             expected = 0;
             digits_seen = 0;

diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h
index 656cf03024714a6fa18bd3afa376e2f29fd2f66b..68ffd6fd50ad11594a64accef30c96f5a8e507ef 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.h
+++ b/src/service_inspectors/http_inspect/http_cutter.h
@@ -125,6 +125,7 @@ private:
     uint32_t data_seen = 0;
     HttpEnums::ChunkState curr_state = HttpEnums::CHUNK_NEWLINES;
     uint32_t expected = 0;
+    uint32_t num_leading_ws = 0;
     uint32_t num_zeros = 0;
     uint32_t digits_seen = 0;
     bool new_section = false;

diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index 562178abc2552e91dfc3531d7bf1b98804590135..0bd4c98ae5b3b6cadc5e57376e926094b85a87ba 100644 (file)
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -71,8 +71,8 @@ enum ScanResult { SCAN_NOTFOUND, SCAN_FOUND, SCAN_FOUND_PIECE, SCAN_DISCARD, SCA
     SCAN_ABORT, SCAN_END };
 
 // State machine for chunk parsing
-enum ChunkState { CHUNK_NEWLINES, CHUNK_ZEROS, CHUNK_NUMBER, CHUNK_WHITESPACE, CHUNK_OPTIONS,
-    CHUNK_HCRLF, CHUNK_DATA, CHUNK_DCRLF1, CHUNK_DCRLF2, CHUNK_BAD };
+enum ChunkState { CHUNK_NEWLINES, CHUNK_ZEROS, CHUNK_LEADING_WS, CHUNK_NUMBER, CHUNK_TRAILING_WS,
+    CHUNK_OPTIONS, CHUNK_HCRLF, CHUNK_DATA, CHUNK_DCRLF1, CHUNK_DCRLF2, CHUNK_BAD };
 
 // List of possible HTTP versions.
 enum VersionId { VERS__NO_SOURCE=-16, VERS__NOT_COMPUTE=-14, VERS__PROBLEMATIC=-12,
@@ -231,6 +231,7 @@ enum Infraction
     INF_CONTENT_ENCODING_CHUNKED,
     INF_206_WITHOUT_RANGE,
     INF_VERSION_NOT_UPPERCASE,
+    INF_CHUNK_LEADING_WS,
     INF__MAX_VALUE
 };
 

diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
index 3df342e0f4c0d6f3a6232dfb23590b33c66c5ae5..b2e58d4fd98df10e2d2c8ba45d9aa85c31d44fab 100644 (file)
--- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
@@ -45,7 +45,9 @@ void HttpStreamSplitter::chunk_spray(HttpFlowData* session_data, uint8_t* buffer
         switch (curr_state)
         {
         case CHUNK_NEWLINES:
-            if (!is_cr_lf[data[k]])
+        case CHUNK_LEADING_WS:
+            // Cases are combined in reassemble(). CHUNK_LEADING_WS here to avoid compiler warning.
+            if (!is_sp_tab_cr_lf[data[k]])
             {
                 curr_state = CHUNK_NUMBER;
                 k--;
@@ -64,13 +66,13 @@ void HttpStreamSplitter::chunk_spray(HttpFlowData* session_data, uint8_t* buffer
             else if (data[k] == ';')
                 curr_state = CHUNK_OPTIONS;
             else if (is_sp_tab[data[k]])
-                curr_state = CHUNK_WHITESPACE;
+                curr_state = CHUNK_TRAILING_WS;
             else
                 expected = expected * 16 + as_hex[data[k]];
             break;
+        case CHUNK_TRAILING_WS:
         case CHUNK_OPTIONS:
-        case CHUNK_WHITESPACE:
-            // No practical difference between white space and options in reassemble()
+            // No practical difference between trailing white space and options in reassemble()
             if (data[k] == '\r')
                 curr_state = CHUNK_HCRLF;
             else if (data[k] == '\n')

diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc
index b9ca3164930b04a2dfe77da6bbe783781e39a91b..9afbfb3563bb829b4968c6c7f8400d229f2dadda 100644 (file)
--- a/src/service_inspectors/http_inspect/http_tables.cc
+++ b/src/service_inspectors/http_inspect/http_tables.cc
@@ -343,7 +343,7 @@ const RuleMap HttpModule::http_events[] =
     { EVENT_URI_BAD_FORMAT,             "URI badly formatted" },
     { EVENT_UNKNOWN_PERCENT,            "unrecognized type of percent encoding in URI" },
     { EVENT_BROKEN_CHUNK,               "HTTP chunk misformatted" },
-    { EVENT_CHUNK_WHITESPACE,           "white space following chunk length" },
+    { EVENT_CHUNK_WHITESPACE,           "white space adjacent to chunk length" },
     { EVENT_HEAD_NAME_WHITESPACE,       "white space within header name" },
     { EVENT_GZIP_OVERRUN,               "excessive gzip compression" },
     { EVENT_GZIP_FAILURE,               "gzip decompression failed" },