From: Tim Kientzle Date: Sat, 15 Dec 2018 20:23:59 +0000 (-0800) Subject: Issue #1055: Ignore padding in Zip extra field data X-Git-Tag: v3.4.0~20^2~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6e2ffe14abb4901e1945895d63e68d65f4694780;p=thirdparty%2Flibarchive.git Issue #1055: Ignore padding in Zip extra field data PKWare's APPNOTE.TXT simply documents the extra field as a concatenation of
objects. It does not explicitly allow nor prohibit additional padding. Libarchive originally treated any extraneous data in the extra field as an indication of corruption and issued an error. However, some Zip writers do appear to include additional zero bytes in the extra field and expect them to be ignored. This works if: * We consider a field type of 0000 to be a padding field. * In particular, 0000 0000 is then a valid field with type zero and zero bytes of data. * If there are fewer than 4 bytes left at the end of the extra data (too few to be a valid extra field object), ignore it. With the above conventions, zero padding can occur anywhere in the extra field as long as the padding occurs in a multiple of four bytes. Any number of bytes can occur at the end: groups of four bytes will be skipped as valid empty fields; any final group of less than four bytes will be simply ignored. The test exercises all of the above options; the extra field data for the file in this archive has: * Four zero bytes at the beginning * A valid UX field * A pad field of type zero with a non-zero length * An additional three final bytes By verifying that the UX data was read correctly and that there were no errors, we can be confident that all of the padding here was handled correctly. --- diff --git a/Makefile.am b/Makefile.am index 80a4e17d1..ea6b2dc00 100644 --- a/Makefile.am +++ b/Makefile.am @@ -512,6 +512,7 @@ libarchive_test_SOURCES= \ libarchive/test/test_read_format_zip_encryption_data.c \ libarchive/test/test_read_format_zip_encryption_partially.c \ libarchive/test/test_read_format_zip_encryption_header.c \ + libarchive/test/test_read_format_zip_extra_padding.c \ libarchive/test/test_read_format_zip_filename.c \ libarchive/test/test_read_format_zip_high_compression.c \ libarchive/test/test_read_format_zip_jar.c \ @@ -839,6 +840,7 @@ libarchive_test_EXTRA_DIST=\ libarchive/test/test_read_format_zip_encryption_data.zip.uu \ libarchive/test/test_read_format_zip_encryption_header.zip.uu \ libarchive/test/test_read_format_zip_encryption_partially.zip.uu \ + libarchive/test/test_read_format_zip_extra_padding.zip.uu \ libarchive/test/test_read_format_zip_filename_cp866.zip.uu \ libarchive/test/test_read_format_zip_filename_cp932.zip.uu \ libarchive/test/test_read_format_zip_filename_koi8r.zip.uu \ diff --git a/libarchive/archive_read_support_format_zip.c b/libarchive/archive_read_support_format_zip.c index 420004dba..55752084c 100644 --- a/libarchive/archive_read_support_format_zip.c +++ b/libarchive/archive_read_support_format_zip.c @@ -750,12 +750,6 @@ process_extra(struct archive_read *a, const char *p, size_t extra_length, struct } offset += datasize; } - if (offset != extra_length) { - archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, - "Malformed extra data: Consumed %d bytes of %d bytes", - (int)offset, (int)extra_length); - return ARCHIVE_FAILED; - } return ARCHIVE_OK; } diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt index 392774857..b18fc8a88 100644 --- a/libarchive/test/CMakeLists.txt +++ b/libarchive/test/CMakeLists.txt @@ -170,6 +170,7 @@ IF(ENABLE_TEST) test_read_format_zip_encryption_data.c test_read_format_zip_encryption_header.c test_read_format_zip_encryption_partially.c + test_read_format_zip_extra_padding.c test_read_format_zip_filename.c test_read_format_zip_high_compression.c test_read_format_zip_jar.c diff --git a/libarchive/test/test_read_format_zip_extra_padding.c b/libarchive/test/test_read_format_zip_extra_padding.c new file mode 100644 index 000000000..8dbb0c5ab --- /dev/null +++ b/libarchive/test/test_read_format_zip_extra_padding.c @@ -0,0 +1,93 @@ +/*- + * Copyright (c) 2003-2018 Tim Kientzle + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR(S) ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR(S) BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +#include "test.h" + +/* + * Test archive verifies that we ignore padding in the extra field. + * + * APPNOTE.txt does not provide any provision for padding the extra + * field, so libarchive used to error when there were unconsumed + * bytes. Apparently, some Zip writers do routinely zero padding + * in the extra field. + * + * The extra fields in this test (for both the local file header + * and the central directory entry) are formatted as follows: + * + * 0000 0000 - unrecognized field with type zero, zero bytes + * 5554 0900 03d258155cdb58155c - UX field with length 9 + * 0000 0400 00000000 - unrecognized field with type zero, four bytes + * 000000 - three bytes padding + * + * The two valid type zero fields should be skipped and ignored, as + * should the three bytes padding (which is too short to be a valid + * extra data object). If there were no errors and we read the UX + * field correctly, then we've correctly handled all of the padding + * fields above. + */ + + +static void verify(struct archive *a) { + struct archive_entry *ae; + + assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae)); + assertEqualString("a", archive_entry_pathname(ae)); + assertEqualInt(AE_IFREG | 0664, archive_entry_mode(ae)); + assertEqualInt(0x5c1558db, archive_entry_mtime(ae)); + assertEqualInt(0, archive_entry_ctime(ae)); + assertEqualInt(0x5c1558db, archive_entry_atime(ae)); + + assertEqualIntA(a, ARCHIVE_EOF, archive_read_next_header(a, &ae)); +} + +DEFINE_TEST(test_read_format_zip_extra_padding) +{ + const char *refname = "test_read_format_zip_extra_padding.zip"; + struct archive *a; + char *p; + size_t s; + + extract_reference_file(refname); + + /* Verify with seeking reader. */ + assert((a = archive_read_new()) != NULL); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_open_filename(a, refname, 7)); + verify(a, 0); + assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a)); + assertEqualInt(ARCHIVE_OK, archive_read_free(a)); + + /* Verify with streaming reader. */ + p = slurpfile(&s, refname); + assert((a = archive_read_new()) != NULL); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a)); + assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a)); + assertEqualIntA(a, ARCHIVE_OK, read_open_memory(a, p, s, 3)); + verify(a, 1); + assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a)); + assertEqualInt(ARCHIVE_OK, archive_read_free(a)); + + free(p); +} diff --git a/libarchive/test/test_read_format_zip_extra_padding.zip.uu b/libarchive/test/test_read_format_zip_extra_padding.zip.uu new file mode 100644 index 000000000..496bd3c49 --- /dev/null +++ b/libarchive/test/test_read_format_zip_extra_padding.zip.uu @@ -0,0 +1,7 @@ +begin 644 test_read_format_zip_extra_padding.zip +M4$L#!`H``````"-=CTW$\L?V`@````(````!`!P`80````!55`D``])8%5S; +M6!5<```$``````````!B"E!+`0(>`PH``````"-=CTW$\L?V`@````(````! +M`!@```````$```"D@0````!A`````%54"0`#TE@57-M8%5P```0``````$L% +3!@`````!``$`1P```#T````````` +` +end