From 87d3c0467298b54c30e0b2f72d50aa5e27784b2d Mon Sep 17 00:00:00 2001 From: Armaan Sandhu Date: Mon, 31 Aug 2026 21:25:37 -0400 Subject: [PATCH] compress: don't grow the output buffer forever on a truncated stream decompress_startswith() decodes just enough of a blob to compare its leading bytes against a prefix, and grows the output buffer whenever a round of decoding came back with less than prefix_len + 1 bytes decoded. That growth is only the right answer while the decoder actually wants more output room. If it instead ran out of input, i.e. the stream is truncated, then no amount of extra room will help, and the gzip and bzip2 paths double their allocation on every iteration until the process is out of memory. Neither decoder lets us tell the two cases apart by return code alone: inflate() reports the stalled stream as Z_BUF_ERROR, which we cannot simply refuse since that is also how it asks for a bigger output buffer, and BZ2_bzDecompress() does not distinguish them at all, it just keeps saying BZ_OK. The output space left over does tell us, though: a decoder that stopped without filling the buffer we gave it is waiting for input that is not coming, so report the stream as corrupt instead of growing. A 10 byte gzip header, or a 4 byte bzip2 header, with nothing behind it was enough to reach a single 4 GiB allocation this way. The xz and zstd paths already terminate, the former because liblzma turns a stalled stream into LZMA_BUF_ERROR. Note that no caller can reach this today: the only non-test caller is decompress_startswith_journal(), and compression_supported_journal() admits only NONE/XZ/LZ4/ZSTD. It is a trap for whoever wires up the other two, so let's close it now. --- src/basic/compress.c | 6 ++++++ src/test/test-compress.c | 13 +++++++++++++ 2 files changed, 19 insertions(+) diff --git a/src/basic/compress.c b/src/basic/compress.c index f3e57dd284..8a3cf92a41 100644 --- a/src/basic/compress.c +++ b/src/basic/compress.c @@ -1440,6 +1440,9 @@ static int decompress_startswith_gzip( if (r == Z_STREAM_END) return 0; + if (s.avail_out > 0) + return -EBADMSG; /* truncated */ + size_t used = allocated - s.avail_out; if (!(greedy_realloc(buffer, allocated * 2, 1))) @@ -1506,6 +1509,9 @@ static int decompress_startswith_bzip2( if (r == BZ_STREAM_END) return 0; + if (s.avail_out > 0) + return -EBADMSG; /* truncated */ + size_t used = allocated - s.avail_out; if (!(greedy_realloc(buffer, allocated * 2, 1))) diff --git a/src/test/test-compress.c b/src/test/test-compress.c index 5c419310b8..e66fb5ce30 100644 --- a/src/test/test-compress.c +++ b/src/test/test-compress.c @@ -341,6 +341,19 @@ TEST(decompress_startswith_short) { ASSERT_OK_POSITIVE(decompress_startswith(c, buf, csize, &buf2, TEXT, i, TEXT[i])); ASSERT_OK_ZERO(decompress_startswith(c, buf, csize, &buf2, TEXT, i, 'y')); } + + /* A truncated stream leaves the decoder unable to make progress: it has consumed all the + * input there is, and the prefix we are after never materialises. Since + * decompress_startswith() grows its output buffer whenever it comes back with less than + * the prefix decoded, that has to be recognised, as more output room cannot substitute + * for missing input. Hence the result may be a match, a non-match, or -EBADMSG, but never + * -ENOMEM. */ + for (size_t i = 1; i < csize; i++) { + _cleanup_free_ void *buf2 = NULL; + + ASSERT_OK_OR(decompress_startswith(c, buf, i, &buf2, TEXT, strlen(TEXT), TEXT[strlen(TEXT)]), + -EBADMSG); + } } #undef TEXT }