From e98b0075e6cb5f7a40e6f19f220b5a55ce733eec Mon Sep 17 00:00:00 2001 From: "Val S." Date: Sun, 12 Oct 2025 19:12:06 -0400 Subject: [PATCH] Fix ZIP parser issue recording empty file entries If csize (and usize) are 0, like with a directory or other empty file entry, then the functionionality to record file record information when indexing the central directory and each associated file record will neglect to store the `local_header_offset` or `local_header_size`. That causes problems later after sorting the file records and then checking for overlapping files. CLAM-2884 --- libclamav/unzip.c | 63 +++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/libclamav/unzip.c b/libclamav/unzip.c index 069ca3796..e9b912da4 100644 --- a/libclamav/unzip.c +++ b/libclamav/unzip.c @@ -717,17 +717,35 @@ static cl_error_t parse_local_file_header( zip += LOCAL_HEADER_elen; bytes_remaining -= LOCAL_HEADER_elen; - if (!csize) { /* FIXME: what's used for method0 files? csize or usize? Nothing in the specs, needs testing */ - cli_dbgmsg("cli_unzip: local header - skipping empty file\n"); - } else { - if (bytes_remaining < csize) { - cli_dbgmsg("cli_unzip: local header - stream out of file\n"); - status = CL_EPARSE; - goto done; - } + if (bytes_remaining < csize) { + cli_dbgmsg("cli_unzip: local header - stream out of file\n"); + status = CL_EPARSE; + goto done; + } + if (NULL != record) { /* Don't actually unzip if we're just collecting the file record information (offset, sizes) */ - if (NULL == record) { + if (NULL == original_filename) { + record->original_filename = NULL; + } else { + record->original_filename = CLI_STRNDUP(original_filename, strlen(original_filename)); + } + record->local_header_offset = loff; + record->local_header_size = zip - local_header; + record->compressed_size = csize; + record->uncompressed_size = usize; + record->method = LOCAL_HEADER_method; + record->flags = LOCAL_HEADER_flags; + record->encrypted = (LOCAL_HEADER_flags & F_ENCR) ? 1 : 0; + + status = CL_SUCCESS; + } else { + /* + * Unzip or decompress & then unzip. + */ + if (!csize) { /* FIXME: what's used for method0 files? csize or usize? Nothing in the specs, needs testing */ + cli_dbgmsg("cli_unzip: local header - skipping empty file\n"); + } else { zip = fmap_need_ptr_once(ctx->fmap, zip, csize); if (NULL == zip) { cli_dbgmsg("cli_unzip: local header - data out of file\n"); @@ -751,27 +769,12 @@ static cl_error_t parse_local_file_header( goto done; } } - } else { - if (NULL == original_filename) { - record->original_filename = NULL; - } else { - record->original_filename = CLI_STRNDUP(original_filename, strlen(original_filename)); - } - record->local_header_offset = loff; - record->local_header_size = zip - local_header; - record->compressed_size = csize; - record->uncompressed_size = usize; - record->method = LOCAL_HEADER_method; - record->flags = LOCAL_HEADER_flags; - record->encrypted = (LOCAL_HEADER_flags & F_ENCR) ? 1 : 0; - - status = CL_SUCCESS; } - - zip += csize; - bytes_remaining -= csize; } + zip += csize; + bytes_remaining -= csize; + if (LOCAL_HEADER_flags & F_USEDD) { if (bytes_remaining < 12) { cli_dbgmsg("cli_unzip: local header - data desc out of file\n"); @@ -910,8 +913,8 @@ static cl_error_t parse_central_directory_file_header( central_header = fmap_need_off(ctx->fmap, central_file_header_offset, SIZEOF_CENTRAL_HEADER); if (NULL == central_header) { - cli_dbgmsg("cli_unzip: central header - file header offset out of file\n"); - status = CL_EPARSE; + cli_dbgmsg("cli_unzip: central header - reached end of central directory.\n"); + status = CL_BREAK; goto done; } @@ -1311,7 +1314,7 @@ cl_error_t index_local_file_headers_within_bounds( if (start_offset > fsize || end_offset > fsize || start_offset > end_offset) { cli_errmsg("index_local_file_headers_within_bounds: Invalid offset arguments: start_offset=%u, end_offset=%u, fsize=%u\n", - start_offset, end_offset, fsize); + start_offset, end_offset, fsize); status = CL_EPARSE; goto done; }