Kafka: Add more loop checks

Add a safeguard to limit the maximum number of iterations.

Do not allocate a new buffer for every loop iterations in a loop that
depends on the result of the decompression routine.

Either allocate the buffer once or free after use. Defensive programming
is more important than speed in this case.

1 files changed, 29 insertions, 9 deletions

diff --git a/epan/dissectors/packet-kafka.c b/epan/dissectors/packet-kafka.c
index 5deb3edce7..99303791f4 100644
--- a/epan/dissectors/packet-kafka.c
+++ b/epan/dissectors/packet-kafka.c
@@ -1615,6 +1615,8 @@ decompress_gzip(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length, t
     }
 }
 
+#define MAX_LOOP_ITERATIONS 100
+
 #ifdef HAVE_LZ4FRAME_H
 static gboolean
 decompress_lz4(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length, tvbuff_t **decompressed_tvb, int *decompressed_offset)
@@ -1674,27 +1676,33 @@ decompress_lz4(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length, tv
         dst_size = (size_t)lz4_info.contentSize;
     }
 
+    decompressed_buffer = wmem_alloc(pinfo->pool, dst_size);
+    size_t out_size;
+    int count = 0;
+
     do {
         src_size = length - src_offset; // set the number of available octets
         if (src_size == 0) {
             goto end;
         }
-        decompressed_buffer = (guchar*)wmem_alloc(pinfo->pool, dst_size);
-        rc = LZ4F_decompress(lz4_ctxt, decompressed_buffer, &dst_size,
+
+        out_size = dst_size;
+        rc = LZ4F_decompress(lz4_ctxt, decompressed_buffer, &out_size,
                               &data[src_offset], &src_size, NULL);
         if (LZ4F_isError(rc)) {
             goto end;
         }
-        if (dst_size == 0) {
+        if (out_size == 0) {
             goto end;
         }
         if (!composite_tvb) {
             composite_tvb = tvb_new_composite();
         }
         tvb_composite_append(composite_tvb,
-                             tvb_new_child_real_data(tvb, (guint8*)decompressed_buffer, (guint)dst_size, (gint)dst_size));
+                             tvb_new_child_real_data(tvb, (guint8*)decompressed_buffer, (guint)out_size, (gint)out_size));
         src_offset += src_size; // bump up the offset for the next iteration
-    } while (rc > 0);
+        DISSECTOR_ASSERT_HINT(count < MAX_LOOP_ITERATIONS, "MAX_LOOP_ITERATIONS exceeded");
+    } while (rc > 0 && count++ < MAX_LOOP_ITERATIONS);
 
     ret = TRUE;
 end:
@@ -1734,8 +1742,9 @@ decompress_snappy(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length,
 
         /* xerial framing format */
         guint32 chunk_size, pos = 16;
+        int count = 0;
 
-        while (pos < length) {
+        while (pos < length && count < MAX_LOOP_ITERATIONS) {
             if (pos > length-4) {
                 // XXX - this is presumably an error, as the chunk size
                 // doesn't fully fit in the data, so an error should be
@@ -1772,6 +1781,9 @@ decompress_snappy(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length,
             tvb_composite_append(composite_tvb,
                       tvb_new_child_real_data(tvb, decompressed_buffer, (guint)uncompressed_size, (gint)uncompressed_size));
             pos += chunk_size;
+            wmem_free(pinfo->pool, decompressed_buffer);
+            count++;
+            DISSECTOR_ASSERT_HINT(count < MAX_LOOP_ITERATIONS, "MAX_LOOP_ITERATIONS exceeded");
         }
 
     } else {
@@ -1825,8 +1837,12 @@ decompress_zstd(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length, t
     size_t rc = 0;
     tvbuff_t *composite_tvb = NULL;
     gboolean ret = FALSE;
-    while (input.pos < input.size) {
-        ZSTD_outBuffer output = { wmem_alloc(pinfo->pool, ZSTD_DStreamOutSize()), ZSTD_DStreamOutSize(), 0 };
+
+    ZSTD_outBuffer output = { wmem_alloc(pinfo->pool, ZSTD_DStreamOutSize()), ZSTD_DStreamOutSize(), 0 };
+    int count = 0;
+
+    while (input.pos < input.size && count < MAX_LOOP_ITERATIONS) {
+
         rc = ZSTD_decompressStream(zds, &output, &input);
         // rc is 0 when a frame is completely decoded and fully flushed,
         // or an error code, which can be tested using ZSTD_isError(),
@@ -1841,7 +1857,11 @@ decompress_zstd(tvbuff_t *tvb, packet_info *pinfo, int offset, guint32 length, t
         }
         tvb_composite_append(composite_tvb,
                              tvb_new_child_real_data(tvb, (guint8*)output.dst, (guint)output.pos, (gint)output.pos));
-        // rc == 0 means there is nothing more to decompress, but there could be still something in the data
+        // Reset output pos to zero to clear the output buffer.
+        output.pos = 0;
+
+        count++;
+        DISSECTOR_ASSERT_HINT(count < MAX_LOOP_ITERATIONS, "MAX_LOOP_ITERATIONS exceeded");
     }
     ret = TRUE;
 end: