From dac2b39405cd3a8bfad8d624202a7e03cdf6c7aa Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 25 Aug 2026 13:15:07 +0900 Subject: [PATCH] compress: strictly check if stream finished on decompress Previously, even when the compressed data was truncated, or had trailing garbage, the decompressor might not detect it. To fix the issue, update the check if stream is finished. With this change, now decompressor_push() must be called with size == 0 to finalize the stream. --- src/basic/compress.c | 195 +++++++++++++++++++++++++++++++-------- src/test/test-compress.c | 59 +++++++++++- 2 files changed, 211 insertions(+), 43 deletions(-) diff --git a/src/basic/compress.c b/src/basic/compress.c index eeecd59b41..f3e57dd284 100644 --- a/src/basic/compress.c +++ b/src/basic/compress.c @@ -133,6 +133,7 @@ static DLSYM_PROTOTYPE(deflateEnd) = NULL; static DLSYM_PROTOTYPE(inflateInit2_) = NULL; static DLSYM_PROTOTYPE(inflate) = NULL; static DLSYM_PROTOTYPE(inflateEnd) = NULL; +static DLSYM_PROTOTYPE(inflateReset) = NULL; static inline void deflateEnd_wrapper(z_stream *s) { sym_deflateEnd(s); @@ -164,6 +165,7 @@ static inline void BZ2_bzDecompressEnd_wrapper(bz_stream *s) { struct Compressor { Compression type; bool encoding; + bool finished; union { #if HAVE_XZ lzma_stream xz; @@ -396,7 +398,8 @@ int dlopen_zlib(int log_level) { DLSYM_ARG(deflateEnd), DLSYM_ARG(inflateInit2_), DLSYM_ARG(inflate), - DLSYM_ARG(inflateEnd)); + DLSYM_ARG(inflateEnd), + DLSYM_ARG(inflateReset)); #else return log_full_errno(log_level, SYNTHETIC_ERRNO(EOPNOTSUPP), "zlib support is not compiled in."); @@ -1814,14 +1817,15 @@ int decompress_stream( n = read(fdf, buf, COMPRESS_PIPE_BUFFER_SIZE); if (n < 0) return -errno; - if (n == 0) - break; total_in += n; r = decompressor_push(c, buf, n, decompress_stream_write_callback, &userdata); if (r < 0) return r; + + if (n == 0) + break; } if (total_in == 0) @@ -1958,15 +1962,20 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress int r; assert(c); + assert(data || size == 0); assert(callback); + /* This feeds `size` bytes of compressed data into `c`. Passing size == 0 has special meaning: it + * signals that no further input will be provided, and the function will attempt to finalize the + * stream, failing with -EBADMSG if the compressed data was truncated or has trailing garbage after + * the logical end of stream. */ + if (c->encoding) return -EINVAL; - if (size == 0) - return 1; - - assert(data); + if (c->finished && size == 0) + /* The previous stream was correctly finished, and it was the last one. */ + return 0; #if HAVE_XZ || HAVE_LZ4 || HAVE_ZSTD || HAVE_ZLIB || HAVE_BZIP2 _cleanup_free_ uint8_t *buffer = NULL; @@ -1980,33 +1989,61 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress switch (c->type) { case COMPRESSION_NONE: + if (c->finished) + return -EBADMSG; + + if (size == 0) { + c->finished = true; + return 0; + } + r = callback(data, size, userdata); if (r < 0) return r; - break; + return 0; #if HAVE_XZ case COMPRESSION_XZ: + if (c->finished) + return -EBADMSG; + c->xz.next_in = data; c->xz.avail_in = size; - while (c->xz.avail_in > 0) { + for (;;) { c->xz.next_out = buffer; c->xz.avail_out = COMPRESS_PIPE_BUFFER_SIZE; - lzma_ret lzr = sym_lzma_code(&c->xz, LZMA_RUN); + lzma_ret lzr = sym_lzma_code(&c->xz, size == 0 ? LZMA_FINISH : LZMA_RUN); if (!IN_SET(lzr, LZMA_OK, LZMA_STREAM_END)) return -EBADMSG; - if (c->xz.avail_out < COMPRESS_PIPE_BUFFER_SIZE) { - r = callback(buffer, COMPRESS_PIPE_BUFFER_SIZE - c->xz.avail_out, userdata); + if (c->xz.avail_out > COMPRESS_PIPE_BUFFER_SIZE) + return -EIO; + + size_t produced = COMPRESS_PIPE_BUFFER_SIZE - c->xz.avail_out; + if (produced > 0) { + r = callback(buffer, produced, userdata); if (r < 0) return r; } - } - break; + if (lzr == LZMA_STREAM_END) { + if (c->xz.avail_in > 0) + return -EBADMSG; /* garbage data at the end */ + + c->finished = true; + return 0; + } + + if (c->xz.avail_in == 0 && c->xz.avail_out > 0) { + if (size == 0) + return -EBADMSG; /* truncated */ + + return 0; /* No progress possible with the current input. */ + } + } #endif #if HAVE_LZ4 @@ -2014,7 +2051,9 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress const uint8_t *src = data; size_t src_remaining = size; - while (src_remaining > 0) { + c->finished = false; + + for (;;) { size_t produced = COMPRESS_PIPE_BUFFER_SIZE; size_t consumed = src_remaining; @@ -2022,8 +2061,9 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress if (sym_LZ4F_isError(rc)) return -EBADMSG; - if (consumed == 0 && produced == 0) - break; /* No progress possible with current input */ + if (consumed > src_remaining || + produced > COMPRESS_PIPE_BUFFER_SIZE) + return -EIO; src += consumed; src_remaining -= consumed; @@ -2033,9 +2073,24 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress if (r < 0) return r; } - } - break; + if (src_remaining == 0) { + if (rc == 0) { + /* The current stream has finished. */ + c->finished = true; + return 0; + } + + if (produced == COMPRESS_PIPE_BUFFER_SIZE) + /* Remaining data exist in the library buffer. We need to read them. */ + continue; + + if (size == 0) + return -EBADMSG; /* truncated */ + + return 0; /* No progress possible with the current input. */ + } + } } #endif @@ -2046,7 +2101,9 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress .size = size, }; - while (input.pos < input.size) { + c->finished = false; + + for (;;) { ZSTD_outBuffer output = { .dst = buffer, .size = COMPRESS_PIPE_BUFFER_SIZE, @@ -2056,14 +2113,33 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress if (sym_ZSTD_isError(res)) return -EBADMSG; + if (input.pos > input.size || + output.pos > output.size) + return -EIO; + if (output.pos > 0) { r = callback(output.dst, output.pos, userdata); if (r < 0) return r; } - } - break; + if (input.pos == input.size) { + if (res == 0) { + /* The current stream has finished. */ + c->finished = true; + return 0; + } + + if (output.pos == COMPRESS_PIPE_BUFFER_SIZE) + /* Remaining data exist in the library buffer. We need to read them. */ + continue; + + if (size == 0) + return -EBADMSG; /* truncated */ + + return 0; /* No progress possible with the current input. */ + } + } } #endif @@ -2075,7 +2151,14 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress c->gzip.next_in = (void*) data; c->gzip.avail_in = size; - while (c->gzip.avail_in > 0) { + for (;;) { + if (c->finished) { + if (sym_inflateReset(&c->gzip) != Z_OK) + return -EIO; + + c->finished = false; + } + c->gzip.next_out = buffer; c->gzip.avail_out = COMPRESS_PIPE_BUFFER_SIZE; @@ -2083,17 +2166,32 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress if (!IN_SET(zr, Z_OK, Z_STREAM_END)) return -EBADMSG; - if (c->gzip.avail_out < COMPRESS_PIPE_BUFFER_SIZE) { - r = callback(buffer, COMPRESS_PIPE_BUFFER_SIZE - c->gzip.avail_out, userdata); + if (c->gzip.avail_out > COMPRESS_PIPE_BUFFER_SIZE) + return -EIO; + + size_t produced = COMPRESS_PIPE_BUFFER_SIZE - c->gzip.avail_out; + if (produced > 0) { + r = callback(buffer, produced, userdata); if (r < 0) return r; } - if (zr == Z_STREAM_END) - break; - } + if (zr == Z_STREAM_END) { + c->finished = true; - break; + if (c->gzip.avail_in > 0) + continue; /* New stream may be pending. */ + + return 0; + } + + if (c->gzip.avail_in == 0 && c->gzip.avail_out > 0) { + if (size == 0) + return -EBADMSG; /* truncated */ + + return 0; /* No progress possible with the current input. */ + } + } #endif #if HAVE_BZIP2 @@ -2104,7 +2202,15 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress c->bzip2.next_in = (char*) data; c->bzip2.avail_in = size; - while (c->bzip2.avail_in > 0) { + for (;;) { + if (c->finished) { + sym_BZ2_bzDecompressEnd(&c->bzip2); + if (sym_BZ2_bzDecompressInit(&c->bzip2, /* verbosity= */ 0, /* small= */ 0) != BZ_OK) + return -EIO; + + c->finished = false; + } + c->bzip2.next_out = (char*) buffer; c->bzip2.avail_out = COMPRESS_PIPE_BUFFER_SIZE; @@ -2112,24 +2218,37 @@ int decompressor_push(Decompressor *c, const void *data, size_t size, Decompress if (!IN_SET(bzr, BZ_OK, BZ_STREAM_END)) return -EBADMSG; - if (c->bzip2.avail_out < COMPRESS_PIPE_BUFFER_SIZE) { - r = callback(buffer, COMPRESS_PIPE_BUFFER_SIZE - c->bzip2.avail_out, userdata); + if (c->bzip2.avail_out > COMPRESS_PIPE_BUFFER_SIZE) + return -EIO; + + size_t produced = COMPRESS_PIPE_BUFFER_SIZE - c->bzip2.avail_out; + if (produced > 0) { + r = callback(buffer, produced, userdata); if (r < 0) return r; } - if (bzr == BZ_STREAM_END) - break; - } + if (bzr == BZ_STREAM_END) { + c->finished = true; - break; + if (c->bzip2.avail_in > 0) + continue; /* New stream may be pending. */ + + return 0; + } + + if (c->bzip2.avail_in == 0 && c->bzip2.avail_out > 0) { + if (size == 0) + return -EBADMSG; /* truncated */ + + return 0; /* No progress possible with the current input. */ + } + } #endif default: assert_not_reached(); } - - return 1; } int compressor_new(Compressor **ret, Compression type) { diff --git a/src/test/test-compress.c b/src/test/test-compress.c index 6ab482f718..5c419310b8 100644 --- a/src/test/test-compress.c +++ b/src/test/test-compress.c @@ -7,6 +7,7 @@ #include "alloc-util.h" #include "argv-util.h" #include "compress.h" +#include "copy.h" #include "fd-util.h" #include "io-util.h" #include "path-util.h" @@ -392,8 +393,7 @@ TEST(compress_decompress_stream) { log_debug("/* test faulty decompression */"); ASSERT_OK_ERRNO(lseek(dst, 1, SEEK_SET)); - r = decompress_stream(c, dst, dst2, st.st_size); - ASSERT_TRUE(IN_SET(r, 0, -EBADMSG)); + ASSERT_ERROR(decompress_stream(c, dst, dst2, st.st_size), EBADMSG); ASSERT_OK_ERRNO(lseek(dst, 0, SEEK_SET)); ASSERT_OK_ERRNO(lseek(dst2, 0, SEEK_SET)); @@ -520,6 +520,51 @@ TEST(decompress_stream_sparse) { compare_fd(src, decompressed); + log_debug("/* testing %s sparse decompression with multiple streams */", compression_to_string(c)); + { + _cleanup_close_ int multi = -EBADF, compressed_multi = -EBADF, decompressed_multi = -EBADF; + _cleanup_(unlink_tempfilep) char + pattern_multi[] = "/tmp/systemd-test.sparse-multi.XXXXXX", + pattern_compressed_multi[] = "/tmp/systemd-test.sparse-compressed_multi.XXXXXX", + pattern_decompressed_multi[] = "/tmp/systemd-test.sparse-decompressed_multi.XXXXXX"; + + ASSERT_OK(multi = mkostemp_safe(pattern_multi)); + ASSERT_OK(compressed_multi = mkostemp_safe(pattern_compressed_multi)); + ASSERT_OK(decompressed_multi = mkostemp_safe(pattern_decompressed_multi)); + + for (unsigned i = 0; i < 4; i++) { + ASSERT_OK(copy_bytes(src, multi, UINT64_MAX, COPY_SEEK0_SOURCE)); + ASSERT_OK(copy_bytes(compressed, compressed_multi, UINT64_MAX, COPY_SEEK0_SOURCE)); + } + + ASSERT_EQ(lseek(compressed_multi, 0, SEEK_SET), (off_t) 0); + ASSERT_OK(decompress_stream(c, compressed_multi, decompressed_multi, UINT64_MAX)); + + compare_fd(multi, decompressed_multi); + } + + log_debug("/* testing %s sparse decompression with garbage at the end */", compression_to_string(c)); + + off_t end = ASSERT_OK_ERRNO(lseek(compressed, 0, SEEK_END)); + ASSERT_OK(loop_write(compressed, "a", 1)); + ASSERT_OK_ERRNO(lseek(compressed, 0, SEEK_SET)); + ASSERT_OK_ERRNO(lseek(decompressed, 0, SEEK_SET)); + ASSERT_ERROR(decompress_stream(c, compressed, decompressed, st_src.st_size), EBADMSG); + + log_debug("/* testing %s sparse decompression with truncated data */", compression_to_string(c)); + + ASSERT_OK_ERRNO(ftruncate(compressed, end - 1)); + ASSERT_OK_ERRNO(lseek(compressed, 0, SEEK_SET)); + ASSERT_OK_ERRNO(lseek(decompressed, 0, SEEK_SET)); + ASSERT_ERROR(decompress_stream(c, compressed, decompressed, st_src.st_size), EBADMSG); + + log_debug("/* testing %s sparse decompression with zero length stream */", compression_to_string(c)); + + ASSERT_OK_ERRNO(ftruncate(compressed, 0)); + ASSERT_OK_ERRNO(lseek(compressed, 0, SEEK_SET)); + ASSERT_OK_ERRNO(lseek(decompressed, 0, SEEK_SET)); + ASSERT_ERROR(decompress_stream(c, compressed, decompressed, st_src.st_size), EBADMSG); + /* Test all-zeros input: entire output should be a hole */ log_debug("/* testing %s sparse decompression of all-zeros */", compression_to_string(c)); { @@ -619,9 +664,13 @@ TEST(compressor_decompressor_push_api) { ASSERT_EQ(compressor_type(decompressor), c); struct decompressor_test_data result = {}; - ASSERT_OK(decompressor_push(decompressor, full_compressed, total_compressed, test_decompressor_callback, &result)); - ASSERT_EQ(result.size, sizeof(text)); - ASSERT_EQ(memcmp(result.buf, text, sizeof(text)), 0); + for (unsigned i = 0; i < 4; i++) + ASSERT_OK(decompressor_push(decompressor, full_compressed, total_compressed, test_decompressor_callback, &result)); + ASSERT_OK(decompressor_push(decompressor, /* data= */ NULL, /* size= */ 0, test_decompressor_callback, &result)); + + ASSERT_EQ(result.size, sizeof(text) * 4); + for (unsigned i = 0; i < 4; i++) + ASSERT_EQ(memcmp(result.buf + sizeof(text) * i, text, sizeof(text)), 0); free(result.buf); decompressor = compressor_free(decompressor);