From adfbd6e0ea3e71b76852cad9936a3d4f2fd4eec0 Mon Sep 17 00:00:00 2001 From: Dieter Baron Date: Wed, 8 Jan 2025 12:41:36 +0100 Subject: [PATCH] Catch read error at end of compressed data. Also, catch garbage after compressed data if checking for consistency. Adresses issue #478 --- lib/zip_algorithm_bzip2.c | 4 +-- lib/zip_algorithm_deflate.c | 4 +-- lib/zip_algorithm_xz.c | 4 +-- lib/zip_algorithm_zstd.c | 4 +-- lib/zip_source_compress.c | 39 ++++++++++++++++++---------- lib/zipint.h | 3 ++- regress/incons-trailing-garbage.zip | Bin 0 -> 193 bytes regress/read_incons.test | 9 +++++++ 8 files changed, 45 insertions(+), 22 deletions(-) create mode 100644 regress/incons-trailing-garbage.zip create mode 100644 regress/read_incons.test diff --git a/lib/zip_algorithm_bzip2.c b/lib/zip_algorithm_bzip2.c index 1818039e8..d5311a721 100644 --- a/lib/zip_algorithm_bzip2.c +++ b/lib/zip_algorithm_bzip2.c @@ -208,11 +208,11 @@ input(void *ud, zip_uint8_t *data, zip_uint64_t length) { } -static void -end_of_input(void *ud) { +static bool end_of_input(void *ud) { struct ctx *ctx = (struct ctx *)ud; ctx->end_of_input = true; + return ctx->zstr.avail_in != 0; } diff --git a/lib/zip_algorithm_deflate.c b/lib/zip_algorithm_deflate.c index 5ab879dfe..402b7e8bc 100644 --- a/lib/zip_algorithm_deflate.c +++ b/lib/zip_algorithm_deflate.c @@ -196,11 +196,11 @@ input(void *ud, zip_uint8_t *data, zip_uint64_t length) { } -static void -end_of_input(void *ud) { +static bool end_of_input(void *ud) { struct ctx *ctx = (struct ctx *)ud; ctx->end_of_input = true; + return ctx->zstr.avail_in != 0; } diff --git a/lib/zip_algorithm_xz.c b/lib/zip_algorithm_xz.c index b0413e013..adadc1cb5 100644 --- a/lib/zip_algorithm_xz.c +++ b/lib/zip_algorithm_xz.c @@ -302,11 +302,11 @@ input(void *ud, zip_uint8_t *data, zip_uint64_t length) { } -static void -end_of_input(void *ud) { +static bool end_of_input(void *ud) { struct ctx *ctx = (struct ctx *)ud; ctx->end_of_input = true; + return ctx->zstr.avail_in != 0; } diff --git a/lib/zip_algorithm_zstd.c b/lib/zip_algorithm_zstd.c index b2aa2132d..21bb2498b 100644 --- a/lib/zip_algorithm_zstd.c +++ b/lib/zip_algorithm_zstd.c @@ -211,11 +211,11 @@ input(void *ud, zip_uint8_t *data, zip_uint64_t length) { } -static void -end_of_input(void *ud) { +static bool end_of_input(void *ud) { struct ctx *ctx = (struct ctx *)ud; ctx->end_of_input = true; + return ctx->in.pos != ctx->in.size; } diff --git a/lib/zip_source_compress.c b/lib/zip_source_compress.c index 54387ecaf..333079616 100644 --- a/lib/zip_source_compress.c +++ b/lib/zip_source_compress.c @@ -44,6 +44,7 @@ struct context { bool can_store; bool is_stored; /* only valid if end_of_stream is true */ bool compress; + bool check_consistency; zip_int32_t method; zip_uint64_t size; @@ -86,11 +87,10 @@ static size_t implementations_size = sizeof(implementations) / sizeof(implementa static zip_source_t *compression_source_new(zip_t *za, zip_source_t *src, zip_int32_t method, bool compress, zip_uint32_t compression_flags); static zip_int64_t compress_callback(zip_source_t *, void *, void *, zip_uint64_t, zip_source_cmd_t); static void context_free(struct context *ctx); -static struct context *context_new(zip_int32_t method, bool compress, zip_uint32_t compression_flags, zip_compression_algorithm_t *algorithm); +static struct context *context_new(zip_int32_t method, bool compress, zip_uint32_t compression_flags, zip_compression_algorithm_t *algorithm, bool check_consistency); static zip_int64_t compress_read(zip_source_t *, struct context *, void *, zip_uint64_t); -zip_compression_algorithm_t * -_zip_get_compression_algorithm(zip_int32_t method, bool compress) { +zip_compression_algorithm_t *_zip_get_compression_algorithm(zip_int32_t method, bool compress) { size_t i; zip_uint16_t real_method = ZIP_CM_ACTUAL(method); @@ -108,16 +108,14 @@ _zip_get_compression_algorithm(zip_int32_t method, bool compress) { return NULL; } -ZIP_EXTERN int -zip_compression_method_supported(zip_int32_t method, int compress) { +ZIP_EXTERN int zip_compression_method_supported(zip_int32_t method, int compress) { if (method == ZIP_CM_STORE) { return 1; } return _zip_get_compression_algorithm(method, compress) != NULL; } -zip_source_t * -zip_source_compress(zip_t *za, zip_source_t *src, zip_int32_t method, zip_uint32_t compression_flags) { +zip_source_t *zip_source_compress(zip_t *za, zip_source_t *src, zip_int32_t method, zip_uint32_t compression_flags) { return compression_source_new(za, src, method, true, compression_flags); } @@ -127,8 +125,7 @@ zip_source_decompress(zip_t *za, zip_source_t *src, zip_int32_t method) { } -static zip_source_t * -compression_source_new(zip_t *za, zip_source_t *src, zip_int32_t method, bool compress, zip_uint32_t compression_flags) { +static zip_source_t *compression_source_new(zip_t *za, zip_source_t *src, zip_int32_t method, bool compress, zip_uint32_t compression_flags) { struct context *ctx; zip_source_t *s2; zip_compression_algorithm_t *algorithm = NULL; @@ -143,7 +140,7 @@ compression_source_new(zip_t *za, zip_source_t *src, zip_int32_t method, bool co return NULL; } - if ((ctx = context_new(method, compress, compression_flags, algorithm)) == NULL) { + if ((ctx = context_new(method, compress, compression_flags, algorithm, za->open_flags & ZIP_CHECKCONS)) == NULL) { zip_error_set(&za->error, ZIP_ER_MEMORY, 0); return NULL; } @@ -157,8 +154,7 @@ compression_source_new(zip_t *za, zip_source_t *src, zip_int32_t method, bool co } -static struct context * -context_new(zip_int32_t method, bool compress, zip_uint32_t compression_flags, zip_compression_algorithm_t *algorithm) { +static struct context *context_new(zip_int32_t method, bool compress, zip_uint32_t compression_flags, zip_compression_algorithm_t *algorithm, bool check_consistency) { struct context *ctx; if ((ctx = (struct context *)malloc(sizeof(*ctx))) == NULL) { @@ -172,6 +168,7 @@ context_new(zip_int32_t method, bool compress, zip_uint32_t compression_flags, z ctx->end_of_input = false; ctx->end_of_stream = false; ctx->is_stored = false; + ctx->check_consistency = check_consistency; if ((ctx->ud = ctx->algorithm->allocate(ZIP_CM_ACTUAL(method), compression_flags, &ctx->error)) == NULL) { zip_error_fini(&ctx->error); @@ -228,7 +225,23 @@ compress_read(zip_source_t *src, struct context *ctx, void *data, zip_uint64_t l ctx->end_of_stream = true; if (!ctx->end_of_input) { - /* TODO: garbage after stream, or compression ended before all data read */ + n = zip_source_read(src, ctx->buffer, 1); + if (n < 0) { + zip_error_set_from_source(&ctx->error, src); + end = true; + break; + } + else if (n == 0) { + ctx->end_of_input = true; + n = ctx->algorithm->end_of_input(ctx->ud) ? 1 : 0; + } + + if (n > 0 && ctx->check_consistency) { + /* garbage after stream, or compression ended before all data read */ + zip_error_set(&ctx->error, ZIP_ER_INCONS, ZIP_ER_DETAIL_COMPRESSED_DATA_TRAILING_GARBAGE); + end = true; + break; + } } if (ctx->first_read < 0) { diff --git a/lib/zipint.h b/lib/zipint.h index 701f4e1de..7f6990ffe 100644 --- a/lib/zipint.h +++ b/lib/zipint.h @@ -153,7 +153,7 @@ struct zip_compression_algorithm { bool (*input)(void *ctx, zip_uint8_t *data, zip_uint64_t length); /* all input data has been provided */ - void (*end_of_input)(void *ctx); + bool (*end_of_input)(void *ctx); /* process input data, writing to data, which has room for length bytes, update length to number of bytes written */ zip_compression_status_t (*process)(void *ctx, zip_uint8_t *data, zip_uint64_t *length); @@ -242,6 +242,7 @@ extern const int _zip_err_details_count; #define ZIP_ER_DETAIL_EOCD64_LOCATOR_MISMATCH 22 /* G EOCD64 and EOCD64 locator do not match */ #define ZIP_ER_DETAIL_UTF8_FILENAME_MISMATCH 23 /* E UTF-8 filename is ASCII and doesn't match filename */ #define ZIP_ER_DETAIL_UTF8_COMMENT_MISMATCH 24 /* E UTF-8 comment is ASCII and doesn't match comment */ +#define ZIP_ER_DETAIL_COMPRESSED_DATA_TRAILING_GARBAGE 25 /* G garbage at end of compressed data */ /* directory entry: general purpose bit flags */ diff --git a/regress/incons-trailing-garbage.zip b/regress/incons-trailing-garbage.zip new file mode 100644 index 0000000000000000000000000000000000000000..2f090353ef6f02f845e2edfa38aa805406b27ddc GIT binary patch literal 193 zcmWIWW@Zs#U|`^2xKit;f6R2JwK$N+3dAf73JfKw#U-I3ybSD)MUD17F?H!(KwMhE z&A`a=l@X`}q*L2dSMMPcLx49s2S__RRC|CoBa;ZT2+&~cMgx_B!Inl4MT|uO-mGjO OwTwU*1f(ND90mZZv?GTA literal 0 HcmV?d00001 diff --git a/regress/read_incons.test b/regress/read_incons.test new file mode 100644 index 000000000..cab43f52b --- /dev/null +++ b/regress/read_incons.test @@ -0,0 +1,9 @@ +arguments -c test.zip cat 0 +return 1 +file test.zip incons-trailing-garbage.zip +stdout +test +end-of-inline-data +stderr +can't read file at index '0': Zip archive inconsistent: garbage at end of compressed data +end-of-inline-data