From: Victor Julien <victor@inliniac.net>
Date: Fri, 13 Dec 2019 21:26:30 +0000 (+0100)
Subject: stream: improve app-layer data retrieval with GAPs
X-Git-Tag: suricata-5.0.2~30
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a742c86741022996cd892819a1a3172288361a89;p=thirdparty%2Fsuricata.git

stream: improve app-layer data retrieval with GAPs

Don't assume that the next block after the sbb head is after the
requested offset.

If the next block was before the offset, the returned data_len
would underflow and return a nonsense value to the app-layer.

Bug #2993.
---

diff --git a/src/stream-tcp-reassemble.c b/src/stream-tcp-reassemble.c
index 7b450db0b7..f8177d54c1 100644
--- a/src/stream-tcp-reassemble.c
+++ b/src/stream-tcp-reassemble.c
@@ -874,6 +874,22 @@ static void GetSessionSize(TcpSession *ssn, Packet *p)
 }
 #endif
 
+static StreamingBufferBlock *GetBlock(StreamingBuffer *sb, const uint64_t offset)
+{
+    StreamingBufferBlock *blk = sb->head;
+    if (blk == NULL)
+        return NULL;
+
+    for ( ; blk != NULL; blk = SBB_RB_NEXT(blk)) {
+        if (blk->offset >= offset)
+            return blk;
+        else if ((blk->offset + blk->len) > offset) {
+            return blk;
+        }
+    }
+    return NULL;
+}
+
 /** \internal
  *
  *  Get buffer, or first part of the buffer if data gaps exist.
@@ -893,33 +909,35 @@ static void GetAppBuffer(TcpStream *stream, const uint8_t **data, uint32_t *data
         *data = mydata;
         *data_len = mydata_len;
     } else {
-        StreamingBufferBlock *blk = stream->sb.head;
+        StreamingBufferBlock *blk = GetBlock(&stream->sb, offset);
+        if (blk == NULL) {
+            *data = NULL;
+            *data_len = 0;
+            return;
+        }
 
-        if (blk->offset > offset) {
+        /* block at expected offset */
+        if (blk->offset == offset) {
+
+            StreamingBufferSBBGetData(&stream->sb, blk, data, data_len);
+
+        /* block past out offset */
+        } else if (blk->offset > offset) {
             SCLogDebug("gap, want data at offset %"PRIu64", "
                     "got data at %"PRIu64". GAP of size %"PRIu64,
                     offset, blk->offset, blk->offset - offset);
             *data = NULL;
             *data_len = blk->offset - offset;
 
-        } else if (offset >= (blk->offset + blk->len)) {
-
-            *data = NULL;
-            StreamingBufferBlock *nblk = SBB_RB_NEXT(blk);
-            *data_len = nblk ? nblk->offset - offset : 0;
-            if (nblk) {
-                SCLogDebug("gap, want data at offset %"PRIu64", "
-                        "got data at %"PRIu64". GAP of size %"PRIu64,
-                        offset, nblk->offset, nblk->offset - offset);
-            }
-
-        } else if (offset > blk->offset && offset < (blk->offset + blk->len)) {
+        /* block starts before offset, but ends after */
+        } else if (offset > blk->offset && offset <= (blk->offset + blk->len)) {
             SCLogDebug("get data from offset %"PRIu64". SBB %"PRIu64"/%u",
                     offset, blk->offset, blk->len);
             StreamingBufferSBBGetDataAtOffset(&stream->sb, blk, data, data_len, offset);
             SCLogDebug("data %p, data_len %u", *data, *data_len);
         } else {
-            StreamingBufferSBBGetData(&stream->sb, blk, data, data_len);
+            *data = NULL;
+            *data_len = 0;
         }
     }
 }