From: Aki Tuomi Date: Wed, 10 Jun 2026 11:54:00 +0000 (+0000) Subject: [PATCH 2/4] lib-compression: Iterate instead of recursing on zero-output decompress... X-Git-Tag: archive/raspbian/1%2.4.1+dfsg1-6+rpi1+deb13u7^2~85 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=361c9d3c790ade51499924ff127123e8d0204d91;p=dovecot.git [PATCH 2/4] lib-compression: Iterate instead of recursing on zero-output decompress chunks 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 702450a..d57955e 100644 --- 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 bb37b85..e141f7d 100644 --- 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 094a9d2..b6fbac1 100644 --- 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 7b3e339..f975278 100644 --- 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