0ccf52a9f
Denys Vlasenko
2016-04-17
Subject
unzip: fix a case where we find wrong CDE. Closes 8821
Binary
No binary (build failed)
Architecture & Compiler Flags
—
Build Log
(log yok)
Commit Diff GitHub →
archival/unzip.c+35 -8
| @@ -45,6 +45,12 @@ | |
| #include "libbb.h" | |
| #include "bb_archive.h" | |
| + | #if 0 |
| + | # define dbg(...) bb_error_msg(__VA_ARGS__) |
| + | #else |
| + | # define dbg(...) ((void)0) |
| + | #endif |
| + | |
| enum { | |
| #if BB_BIG_ENDIAN | |
| ZIP_FILEHEADER_MAGIC = 0x504b0304, | |
| @@ -193,15 +199,17 @@ static uint32_t find_cdf_offset(void) | |
| unsigned char *p; | |
| off_t end; | |
| unsigned char *buf = xzalloc(PEEK_FROM_END); | |
| + | uint32_t found; |
| end = xlseek(zip_fd, 0, SEEK_END); | |
| end -= PEEK_FROM_END; | |
| if (end < 0) | |
| end = 0; | |
| - | xlseek(zip_fd, end, SEEK_SET); |
| + | dbg("Looking for cdf_offset starting from 0x%"OFF_FMT"x", end); |
| + | xlseek(zip_fd, end, SEEK_SET); |
| full_read(zip_fd, buf, PEEK_FROM_END); | |
| - | cde_header.formatted.cdf_offset = BAD_CDF_OFFSET; |
| + | found = BAD_CDF_OFFSET; |
| p = buf; | |
| while (p <= buf + PEEK_FROM_END - CDE_HEADER_LEN - 4) { | |
| if (*p != 'P') { | |
| @@ -220,14 +228,25 @@ static uint32_t find_cdf_offset(void) | |
| /* | |
| * I've seen .ZIP files with seemingly valid CDEs | |
| * where cdf_offset points past EOF - ?? | |
| - | * Ignore such CDEs: |
| + | * This check ignores such CDEs: |
| */ | |
| - | if (cde_header.formatted.cdf_offset < end + (p - buf)) |
| - | break; |
| - | cde_header.formatted.cdf_offset = BAD_CDF_OFFSET; |
| + | if (cde_header.formatted.cdf_offset < end + (p - buf)) { |
| + | found = cde_header.formatted.cdf_offset; |
| + | dbg("Possible cdf_offset:0x%x at 0x%"OFF_FMT"x", |
| + | (unsigned)found, end + (p-3 - buf)); |
| + | dbg(" cdf_offset+cdf_size:0x%x", |
| + | (unsigned)(found + SWAP_LE32(cde_header.formatted.cdf_size))); |
| + | /* |
| + | * We do not "break" here because only the last CDE is valid. |
| + | * I've seen a .zip archive which contained a .zip file, |
| + | * uncompressed, and taking the first CDE was using |
| + | * the CDE inside that file! |
| + | */ |
| + | } |
| } | |
| free(buf); | |
| - | return cde_header.formatted.cdf_offset; |
| + | dbg("Found cdf_offset:0x%x", (unsigned)found); |
| + | return found; |
| }; | |
| static uint32_t read_next_cdf(uint32_t cdf_offset, cdf_header_t *cdf_ptr) | |
| @@ -240,9 +259,13 @@ static uint32_t read_next_cdf(uint32_t cdf_offset, cdf_header_t *cdf_ptr) | |
| cdf_offset = find_cdf_offset(); | |
| if (cdf_offset != BAD_CDF_OFFSET) { | |
| + | dbg("Reading CDF at 0x%x", (unsigned)cdf_offset); |
| xlseek(zip_fd, cdf_offset + 4, SEEK_SET); | |
| xread(zip_fd, cdf_ptr->raw, CDF_HEADER_LEN); | |
| FIX_ENDIANNESS_CDF(*cdf_ptr); | |
| + | dbg("file_name_length:%u", (unsigned)cdf_ptr->formatted.file_name_length); |
| + | dbg("extra_field_length:%u", (unsigned)cdf_ptr->formatted.extra_field_length); |
| + | dbg("file_comment_length:%u", (unsigned)cdf_ptr->formatted.file_comment_length); |
| cdf_offset += 4 + CDF_HEADER_LEN | |
| + cdf_ptr->formatted.file_name_length | |
| + cdf_ptr->formatted.extra_field_length | |
| @@ -532,11 +555,14 @@ int unzip_main(int argc, char **argv) | |
| /* Check magic number */ | |
| xread(zip_fd, &magic, 4); | |
| /* Central directory? It's at the end, so exit */ | |
| - | if (magic == ZIP_CDF_MAGIC) |
| + | if (magic == ZIP_CDF_MAGIC) { |
| + | dbg("got ZIP_CDF_MAGIC"); |
| break; | |
| + | } |
| #if ENABLE_DESKTOP | |
| /* Data descriptor? It was a streaming file, go on */ | |
| if (magic == ZIP_DD_MAGIC) { | |
| + | dbg("got ZIP_DD_MAGIC"); |
| /* skip over duplicate crc32, cmpsize and ucmpsize */ | |
| unzip_skip(3 * 4); | |
| continue; | |
| @@ -544,6 +570,7 @@ int unzip_main(int argc, char **argv) | |
| #endif | |
| if (magic != ZIP_FILEHEADER_MAGIC) | |
| bb_error_msg_and_die("invalid zip magic %08X", (int)magic); | |
| + | dbg("got ZIP_FILEHEADER_MAGIC"); |
| /* Read the file header */ | |
| xread(zip_fd, zip_header.raw, ZIP_HEADER_LEN); | |