[PATCH 2/4] lib-compression: Iterate instead of recursing on zero-output decompress...

istream-lz4, istream-zlib and istream-bzlib each retried a read that
produced no output by tail-calling their own read function. A crafted
compressed stream can contain an unbounded number of chunks/steps that
each decompress to zero bytes - e.g. an lz4 stream of single-byte chunks
that each decode to nothing - so an attacker controlling the compressed
data can drive recursion depth proportional to the chunk count. Tail-call
optimization is not guaranteed, so this can exhaust the stack and crash
the process reading the stream.

Replace the recursive calls with continue inside the for(;;) loop
introduced in the previous commit, keeping stack usage O(1). Add a
regression test that feeds istream-lz4 a stream of 100000 empty chunks
and reads it in a single call.

Gbp-Pq: Name 0002-lib-compression-Iterate-instead-of-recursing-on-zero.patch

diff --git a/src/lib-compression/istream-bzlib.c b/src/lib-compression/istream-bzlib.c
index 702450aba90917a3e39152685a33a4d2235e079e..d57955e727c00a558f05b7022172786c5e13a889 100644 (file)
--- a/src/lib-compression/istream-bzlib.c
+++ b/src/lib-compression/istream-bzlib.c
@@ -51,6 +51,10 @@ static ssize_t i_stream_bzlib_read(struct istream_private *stream)
        size_t size, out_size;
        int ret;
 
+       /* Loop instead of recursing when a decompress step yields no output:
+          a crafted stream can produce an unbounded number of zero-output
+          steps, and tail-call recursion (not guaranteed) would exhaust the
+          stack. */
        for (;;) {
                high_offset = stream->istream.v_offset + (stream->pos - stream->skip);
                if (zstream->eof_offset == high_offset) {
@@ -130,8 +134,8 @@ static ssize_t i_stream_bzlib_read(struct istream_private *stream)
                        i_fatal("BZ2_bzDecompress() failed with %d", ret);
                }
                if (out_size == 0) {
-                       /* read more input */
-                       return i_stream_bzlib_read(stream);
+                       /* no output yet; read more input without recursing */
+                       continue;
                }
                return out_size;
        }

diff --git a/src/lib-compression/istream-lz4.c b/src/lib-compression/istream-lz4.c
index bb37b8541f54a34bd302ebbb71cd4a303e13ec9b..e141f7df8e23005339e6229e57c0b665ed48c79b 100644 (file)
--- a/src/lib-compression/istream-lz4.c
+++ b/src/lib-compression/istream-lz4.c
@@ -149,6 +149,10 @@ static ssize_t i_stream_lz4_read(struct istream_private *stream)
                zstream->header_read = TRUE;
        }
 
+       /* Loop over chunks instead of recursing on zero-output chunks: a
+          crafted stream can contain an unbounded number of chunks that each
+          decompress to zero bytes, and tail-call recursion (which is not
+          guaranteed) would let that exhaust the stack. */
        for (;;) {
                if (zstream->chunk_left == 0) {
                        while ((ret = i_stream_lz4_read_chunk_header(zstream)) == 0) {
@@ -183,8 +187,8 @@ static ssize_t i_stream_lz4_read(struct istream_private *stream)
                if (stream->pos - stream->skip >= i_stream_get_max_buffer_size(&stream->istream))
                        return -2;
                if (i_stream_get_data_size(zstream->istream.parent) > 0) {
-                       /* Parent stream was only partially consumed. Set the stream's
-                          IO as pending to avoid hangs. */
+                       /* Parent stream was only partially consumed. Set the
+                          stream's IO as pending to avoid hangs. */
                        i_stream_set_input_pending(&zstream->istream.istream, TRUE);
                }
                /* allocate enough space for the old data and the new
@@ -199,8 +203,12 @@ static ssize_t i_stream_lz4_read(struct istream_private *stream)
                        lz4_read_error(zstream, "corrupted lz4 chunk");
                        stream->istream.stream_errno = EINVAL;
                        return -1;
-               } else if (ret == 0)
-                       return i_stream_lz4_read(stream);
+               } else if (ret == 0) {
+                       /* chunk decompressed to zero bytes; read the next chunk
+                          without recursing so the stack stays bounded. */
+                       buffer_set_used_size(zstream->chunk_buf, 0);
+                       continue;
+               }
                i_assert(ret > 0);
                stream->pos += ret;
                i_assert(stream->pos <= stream->buffer_size);

diff --git a/src/lib-compression/istream-zlib.c b/src/lib-compression/istream-zlib.c
index 094a9d2a65be4bbcbfd33333904cf7a4cd41c565..b6fbac1af92c13b68d662f090bd2911118a403e0 100644 (file)
--- a/src/lib-compression/istream-zlib.c
+++ b/src/lib-compression/istream-zlib.c
@@ -168,6 +168,10 @@ static ssize_t i_stream_zlib_read(struct istream_private *stream)
        size_t size, out_size;
        int ret;
 
+       /* Loop instead of recursing when an inflate step yields no output:
+          a crafted stream can produce an unbounded number of zero-output
+          steps, and tail-call recursion (not guaranteed) would exhaust the
+          stack. */
        for (;;) {
                high_offset = stream->istream.v_offset + (stream->pos - stream->skip);
                if (zstream->eof_offset == high_offset) {
@@ -314,8 +318,8 @@ static ssize_t i_stream_zlib_read(struct istream_private *stream)
                        i_fatal("inflate() failed with %d", ret);
                }
                if (out_size == 0) {
-                       /* read more input */
-                       return i_stream_zlib_read(stream);
+                       /* no output yet; read more input without recursing */
+                       continue;
                }
                return out_size;
        }

diff --git a/src/lib-compression/test-compression.c b/src/lib-compression/test-compression.c
index 7b3e33980b4e91e5ce4aef01645b1ca4d463758e..f97527892aee21b503748b42b5c6564b897c3562 100644 (file)
--- a/src/lib-compression/test-compression.c
+++ b/src/lib-compression/test-compression.c
@@ -1108,6 +1108,55 @@ static void test_lz4_chunk_size(void)
        test_end();
 }
 
+static void test_lz4_many_empty_chunks(void)
+{
+       const struct compression_handler *lz4;
+       struct istream *file_input, *input;
+
+       if (compression_lookup_handler("lz4", &lz4) <= 0)
+               return; /* not compiled in or unknown */
+
+       test_begin("lz4 many empty chunks");
+
+       /* A crafted lz4 stream can contain an unbounded number of chunks that
+          each decompress to zero bytes. i_stream_lz4_read() must iterate over
+          them rather than recurse once per chunk, or the stack is exhausted
+          (tail-call optimization is not guaranteed). */
+       buffer_t *buf = buffer_create_dynamic(default_pool, 1024*512);
+       struct iostream_lz4_header hdr;
+       memcpy(hdr.magic, IOSTREAM_LZ4_MAGIC, IOSTREAM_LZ4_MAGIC_LEN);
+       /* a valid (64k) max uncompressed chunk size, big-endian */
+       hdr.max_uncompressed_chunk_size[0] = 0x00;
+       hdr.max_uncompressed_chunk_size[1] = 0x01;
+       hdr.max_uncompressed_chunk_size[2] = 0x00;
+       hdr.max_uncompressed_chunk_size[3] = 0x00;
+       buffer_append(buf, &hdr, sizeof(hdr));
+
+       /* Each chunk: 4-byte big-endian compressed length (1), then a single
+          0x00 byte, which LZ4_decompress_safe() decodes to zero bytes. */
+       static const unsigned char empty_chunk[] = {
+               0x00, 0x00, 0x00, 0x01, 0x00
+       };
+       for (unsigned int i = 0; i < 100000; i++)
+               buffer_append(buf, empty_chunk, sizeof(empty_chunk));
+
+       file_input = test_istream_create_data(buf->data, buf->used);
+       file_input->blocking = TRUE;
+       input = lz4->create_istream(file_input);
+       i_stream_unref(&file_input);
+
+       /* All chunks are empty: clean EOF, no content, no error - and, with
+          the iterative read, no stack overflow. */
+       test_assert(i_stream_read(input) == -1);
+       test_assert(input->eof);
+       test_assert(input->stream_errno == 0);
+
+       i_stream_unref(&input);
+       buffer_free(&buf);
+
+       test_end();
+}
+
 static void test_uncompress_file(const char *path)
 {
        const struct compression_handler *handler;
@@ -1232,6 +1281,7 @@ int main(int argc, char *argv[])
                test_gz_large_header,
                test_lz4_small_header,
                test_lz4_chunk_size,
+               test_lz4_many_empty_chunks,
                test_compression_ext,
                test_compression_deinit,
                NULL