Skip to content

Commit

Permalink
Catch read error at end of compressed data.
Browse files Browse the repository at this point in the history
Also, catch garbage after compressed data if checking for consistency.

Adresses issue #478
  • Loading branch information
dillof committed Jan 8, 2025
1 parent b57ed83 commit adfbd6e
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 22 deletions.
4 changes: 2 additions & 2 deletions lib/zip_algorithm_bzip2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}


Expand Down
4 changes: 2 additions & 2 deletions lib/zip_algorithm_deflate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}


Expand Down
4 changes: 2 additions & 2 deletions lib/zip_algorithm_xz.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}


Expand Down
4 changes: 2 additions & 2 deletions lib/zip_algorithm_zstd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}


Expand Down
39 changes: 26 additions & 13 deletions lib/zip_source_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}

Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion lib/zipint.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 */

Expand Down
Binary file added regress/incons-trailing-garbage.zip
Binary file not shown.
9 changes: 9 additions & 0 deletions regress/read_incons.test
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit adfbd6e

Please sign in to comment.