SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

compress: strictly check if stream finished on decompress

Browse Source 
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.
This commit is contained in:
Yu Watanabe
2026-08-27 04:36:57 +09:00
parent d9b6e8663c
commit dac2b39405
2 changed files with 211 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/compress.c
+157 -38
View File
@@ -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) {
src/test/test-compress.c
+54 -5
View File
@@ -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);