From: Tom Peters (thopeter) <thopeter@cisco.com>
Date: Mon, 11 Jun 2018 18:58:04 +0000 (-0400)
Subject: Merge pull request #1265 in SNORT/snort3 from nhttp107 to master
X-Git-Tag: 3.0.0-246~62
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c102fa2bb0fc5632504b75992d2b0d30027b617a;p=thirdparty%2Fsnort3.git

Merge pull request #1265 in SNORT/snort3 from nhttp107 to master

Squashed commit of the following:

commit f75afd52d4ec9c58c50f08e3cee88fb70f92f94c
Author: Tom Peters <thopeter@cisco.com>
Date:   Mon Jun 11 12:09:15 2018 -0400

    http_inspect: bug fix and cleanup
---

diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index 7eace0b28..6c789828f 100644
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -345,6 +345,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 {
                     events->create_event(EVENT_BROKEN_CHUNK);
                     curr_state = CHUNK_BAD;
+                    k--;
                 }
                 break;
             }
@@ -397,6 +398,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 *infractions += INF_CHUNK_BAD_CHAR;
                 events->create_event(EVENT_BROKEN_CHUNK);
                 curr_state = CHUNK_BAD;
+                k--;
             }
             else
             {
@@ -407,6 +409,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                     *infractions += INF_CHUNK_TOO_LARGE;
                     events->create_event(EVENT_BROKEN_CHUNK);
                     curr_state = CHUNK_BAD;
+                    k--;
                 }
             }
             break;
@@ -435,6 +438,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 *infractions += INF_CHUNK_BAD_CHAR;
                 events->create_event(EVENT_BROKEN_CHUNK);
                 curr_state = CHUNK_BAD;
+                k--;
             }
             break;
         case CHUNK_OPTIONS:
@@ -462,6 +466,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 *infractions += INF_CHUNK_LONE_CR;
                 events->create_event(EVENT_BROKEN_CHUNK);
                 curr_state = CHUNK_BAD;
+                k--;
                 break;
             }
             if (expected > 0)
@@ -480,6 +485,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 *infractions += INF_CHUNK_NO_LENGTH;
                 events->create_event(EVENT_BROKEN_CHUNK);
                 curr_state = CHUNK_BAD;
+                k--;
             }
             break;
         case CHUNK_DATA:
@@ -497,7 +503,6 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
             }
             if ((data_seen += skip_amount) == flow_target)
             {
-                // FIXIT-M need to randomize slice point
                 data_seen = 0;
                 num_flush = k+1;
                 new_section = true;
@@ -523,6 +528,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 *infractions += INF_CHUNK_BAD_END;
                 events->create_event(EVENT_BROKEN_CHUNK);
                 curr_state = CHUNK_BAD;
+                k--;
             }
             break;
         case CHUNK_DCRLF2:
@@ -553,13 +559,19 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
                 num_flush = length;
                 return SCAN_DISCARD_PIECE;
             }
+
+            // When chunk parsing breaks down and we first enter CHUNK_BAD state, it may happen
+            // that there were chunk header bytes between the last good chunk and the point where
+            // the failure occurred. These will not have been counted in data_seen because we
+            // planned to delete them during reassembly. Because they are not part of a valid chunk
+            // they will be reassembled after all. This will overrun the flow_target making the
+            // message section a little bigger than planned. It's not important.
             uint32_t skip_amount = length-k;
             skip_amount = (skip_amount <= flow_target-data_seen) ? skip_amount :
                 flow_target-data_seen;
             k += skip_amount - 1;
             if ((data_seen += skip_amount) == flow_target)
             {
-                // FIXIT-M need to randomize slice point
                 data_seen = 0;
                 num_flush = k+1;
                 new_section = true;
@@ -573,6 +585,7 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
         num_flush = length;
         return SCAN_DISCARD_PIECE;
     }
+
     octets_seen += length;
     return SCAN_NOTFOUND;
 }