]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
ips/stream: handle low mem(cap) crash
authorVictor Julien <victor@inliniac.net>
Fri, 22 Feb 2019 19:41:41 +0000 (20:41 +0100)
committerVictor Julien <victor@inliniac.net>
Sun, 24 Feb 2019 19:00:55 +0000 (20:00 +0100)
In low memory or memcap reached conditions a crash could happen in
inline stream detection.

The crash had the following path:

A packet would come in and it's data was added to the stream. Due
to earlier packet loss, the stream buffer uses a stream buffer block
tree to track the data blocks. When trying to add the current packets
block to the tree, the memory limit was reached and the add fails.

A bit later in the pipeline for the same packet, the inline stream
mpm inspection function gets the data to inspect. For inline mode
this is the current packet + stream data before and after the packet,
if available.

The code looking up the packets data in the stream would not
consider the possibility that the stream block returned wasn't
the right one. The tree search returns either the correct or the
next block. In adjusting the returned block to add the extra stream
data it would miscalculate offsets leading to a corrupt pointer to the
data.

This patch more carefully checks the result of the lookup, and
falls back to simply inspecting the packet payload if the lookup
didn't produce the expected result.

Bug 2842.

Reported-by: Ad Schellevis <ad@opnsense.org>
src/stream-tcp-reassemble.c

index f5ece6f38c811b7925848b158b690b7bbd9ef2cc..155f685e262bcf906ce1876a3161a6d3e28c5e09 100644 (file)
@@ -1400,69 +1400,68 @@ static int StreamReassembleRawInline(TcpSession *ssn, const Packet *p,
     }
 
     /* this can only happen if the segment insert of our current 'p' failed */
-    if (mydata == NULL || mydata_len == 0) {
-        SCLogDebug("no data: %p/%u", mydata, mydata_len);
-        *progress_out = STREAM_RAW_PROGRESS(stream);
-        return 0;
-    }
-    if (mydata_offset >= packet_rightedge_abs ||
-        (packet_leftedge_abs >= (mydata_offset + mydata_len))) {
-        SCLogDebug("no data overlap: %p/%u", mydata, mydata_len);
-        *progress_out = STREAM_RAW_PROGRESS(stream);
-        return 0;
-    }
-
-    /* adjust buffer to match chunk_size */
-
     uint64_t mydata_rightedge_abs = mydata_offset + mydata_len;
-    SCLogDebug("chunk_size %u mydata_len %u", chunk_size, mydata_len);
-    if (mydata_len > chunk_size) {
-        uint32_t excess = mydata_len - chunk_size;
-        SCLogDebug("chunk_size %u mydata_len %u excess %u", chunk_size, mydata_len, excess);
-
-        if (mydata_rightedge_abs == packet_rightedge_abs) {
-            mydata += excess;
-            mydata_len -= excess;
-            mydata_offset += excess;
-            SCLogDebug("cutting front of the buffer with %u", excess);
-        } else if (mydata_offset == packet_leftedge_abs) {
-            mydata_len -= excess;
-            SCLogDebug("cutting tail of the buffer with %u", excess);
-        } else {
-            uint32_t before = (uint32_t)(packet_leftedge_abs - mydata_offset);
-            uint32_t after = (uint32_t)(mydata_rightedge_abs - packet_rightedge_abs);
-            SCLogDebug("before %u after %u", before, after);
-
-            if (after >= (chunk_size - p->payload_len) / 2) {
-                // more trailing data than we need
-
-                if (before >= (chunk_size - p->payload_len) / 2) {
-                    // also more heading data, devide evenly
-                    before = after = (chunk_size - p->payload_len) / 2;
-                } else {
-                    // heading data is less than requested, give the
-                    // rest to the trailing data
-                    after = (chunk_size - p->payload_len) - before;
-                }
+    if ((mydata == NULL || mydata_len == 0) || /* no data */
+            (mydata_offset >= packet_rightedge_abs || /* data all to the right */
+             packet_leftedge_abs >= mydata_rightedge_abs) || /* data all to the left */
+            (packet_leftedge_abs < mydata_offset || /* data missing at the start */
+             packet_rightedge_abs > mydata_rightedge_abs)) /* data missing at the end */
+    {
+        /* no data, or data is incomplete or wrong: use packet data */
+        mydata = p->payload;
+        mydata_offset = packet_leftedge_abs;
+        //mydata_rightedge_abs = packet_rightedge_abs;
+    } else {
+        /* adjust buffer to match chunk_size */
+        SCLogDebug("chunk_size %u mydata_len %u", chunk_size, mydata_len);
+        if (mydata_len > chunk_size) {
+            uint32_t excess = mydata_len - chunk_size;
+            SCLogDebug("chunk_size %u mydata_len %u excess %u", chunk_size, mydata_len, excess);
+
+            if (mydata_rightedge_abs == packet_rightedge_abs) {
+                mydata += excess;
+                mydata_len -= excess;
+                mydata_offset += excess;
+                SCLogDebug("cutting front of the buffer with %u", excess);
+            } else if (mydata_offset == packet_leftedge_abs) {
+                mydata_len -= excess;
+                SCLogDebug("cutting tail of the buffer with %u", excess);
             } else {
-                // less trailing data than requested
-
-                if (before >= (chunk_size - p->payload_len) / 2) {
-                    before = (chunk_size - p->payload_len) - after;
+                uint32_t before = (uint32_t)(packet_leftedge_abs - mydata_offset);
+                uint32_t after = (uint32_t)(mydata_rightedge_abs - packet_rightedge_abs);
+                SCLogDebug("before %u after %u", before, after);
+
+                if (after >= (chunk_size - p->payload_len) / 2) {
+                    // more trailing data than we need
+
+                    if (before >= (chunk_size - p->payload_len) / 2) {
+                        // also more heading data, devide evenly
+                        before = after = (chunk_size - p->payload_len) / 2;
+                    } else {
+                        // heading data is less than requested, give the
+                        // rest to the trailing data
+                        after = (chunk_size - p->payload_len) - before;
+                    }
                 } else {
-                    // both smaller than their requested size
+                    // less trailing data than requested
+
+                    if (before >= (chunk_size - p->payload_len) / 2) {
+                        before = (chunk_size - p->payload_len) - after;
+                    } else {
+                        // both smaller than their requested size
+                    }
                 }
-            }
 
-            /* adjust the buffer */
-            uint32_t skip = (uint32_t)(packet_leftedge_abs - mydata_offset) - before;
-            uint32_t cut = (uint32_t)(mydata_rightedge_abs - packet_rightedge_abs) - after;
-            DEBUG_VALIDATE_BUG_ON(skip > mydata_len);
-            DEBUG_VALIDATE_BUG_ON(cut > mydata_len);
-            DEBUG_VALIDATE_BUG_ON(skip + cut > mydata_len);
-            mydata += skip;
-            mydata_len -= (skip + cut);
-            mydata_offset += skip;
+                /* adjust the buffer */
+                uint32_t skip = (uint32_t)(packet_leftedge_abs - mydata_offset) - before;
+                uint32_t cut = (uint32_t)(mydata_rightedge_abs - packet_rightedge_abs) - after;
+                DEBUG_VALIDATE_BUG_ON(skip > mydata_len);
+                DEBUG_VALIDATE_BUG_ON(cut > mydata_len);
+                DEBUG_VALIDATE_BUG_ON(skip + cut > mydata_len);
+                mydata += skip;
+                mydata_len -= (skip + cut);
+                mydata_offset += skip;
+            }
         }
     }