ZIP reader: fix a crash in PPMd8 decompressor.

The crash happened on invalid files which declare more data than
actually are stored in the file.

This commit contains a fix that prevents PPMd8 decompressor from
crashing, as well as relevant tests containing files that were
triggering the crash.

diff --git a/libarchive/archive_read_support_format_zip.c b/libarchive/archive_read_support_format_zip.c
index b91b66f6b7592686997bdcc617920a9d02c8ef67..723ea1e04c3e46c8f711e39dd6b0d846582b0f37 100644 (file)
--- a/libarchive/archive_read_support_format_zip.c
+++ b/libarchive/archive_read_support_format_zip.c
@@ -194,6 +194,7 @@ struct zip {
        ssize_t                 zipx_ppmd_read_compressed;
        CPpmd8                  ppmd8;
        char                    ppmd8_valid;
+       char                    ppmd8_stream_failed;
 
        struct archive_string_conv *sconv;
        struct archive_string_conv *sconv_default;
@@ -254,9 +255,15 @@ ppmd_read(void* p) {
        /* Get the handle to current decompression context. */
        struct archive_read *a = ((IByteIn*)p)->a;
        struct zip *zip = (struct zip*) a->format->data;
+       ssize_t bytes_avail = 0;
 
        /* Fetch next byte. */
-       const uint8_t* data = __archive_read_ahead(a, 1, NULL);
+       const uint8_t* data = __archive_read_ahead(a, 1, &bytes_avail);
+       if(bytes_avail < 1) {
+               zip->ppmd8_stream_failed = 1;
+               return 0;
+       }
+
        __archive_read_consume(a, 1);
 
        /* Increment the counter. */
@@ -1750,6 +1757,7 @@ zipx_ppmd8_init(struct archive_read *a, struct zip *zip)
 
        /* Create a new decompression context. */
        __archive_ppmd8_functions.Ppmd8_Construct(&zip->ppmd8);
+       zip->ppmd8_stream_failed = 0;
 
        /* Setup function pointers required by Ppmd8 decompressor. The
         * 'ppmd_read' function will feed new bytes to the decompressor,
@@ -1869,6 +1877,14 @@ zip_read_data_zipx_ppmd(struct archive_read *a, const void **buff,
                        break;
                }
 
+               /* This field is set by ppmd_read() when there was no more data
+                * to be read. */
+               if(zip->ppmd8_stream_failed) {
+                       archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
+                               "Truncated PPMd8 file body");
+                       return (ARCHIVE_FATAL);
+               }
+
                zip->uncompressed_buffer[consumed_bytes] = (uint8_t) sym;
                ++consumed_bytes;
        } while(consumed_bytes < zip->uncompressed_buffer_size);

diff --git a/libarchive/test/test_read_format_zip.c b/libarchive/test/test_read_format_zip.c
index d152a19b4f847f2f41fdc59da0d96d792ac1a204..bc04933c3ea320e9eca57e5be596250ce7ece086 100644 (file)
--- a/libarchive/test/test_read_format_zip.c
+++ b/libarchive/test/test_read_format_zip.c
@@ -759,3 +759,45 @@ DEFINE_TEST(test_read_format_zip_xz_multi_blockread)
        assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
        assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a));
 }
+
+DEFINE_TEST(test_read_format_zip_ppmd8_crash_1)
+{
+       const char *refname = "test_read_format_zip_ppmd8_crash_2.zipx";
+       struct archive *a;
+       struct archive_entry *ae;
+       char buf[64];
+
+       extract_reference_file(refname);
+
+       assert((a = archive_read_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_zip(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_open_filename(a, refname, 37));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
+
+       /* The file `refname` is invalid in this case, so this call should fail.
+        * But it shouldn't crash. */
+       assertEqualIntA(a, ARCHIVE_FATAL, archive_read_data(a, buf, 64));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a));
+}
+
+DEFINE_TEST(test_read_format_zip_ppmd8_crash_2)
+{
+       const char *refname = "test_read_format_zip_ppmd8_crash_2.zipx";
+       struct archive *a;
+       struct archive_entry *ae;
+       char buf[64];
+
+       extract_reference_file(refname);
+
+       assert((a = archive_read_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_zip(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_open_filename(a, refname, 37));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
+
+       /* The file `refname` is invalid in this case, so this call should fail.
+        * But it shouldn't crash. */
+       assertEqualIntA(a, ARCHIVE_FATAL, archive_read_data(a, buf, 64));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_close(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_free(a));
+}

diff --git a/libarchive/test/test_read_format_zip_ppmd8_crash_1.zipx.uu b/libarchive/test/test_read_format_zip_ppmd8_crash_1.zipx.uu
new file mode 100644 (file)
index 0000000..fb6050f
--- /dev/null
+++ b/libarchive/test/test_read_format_zip_ppmd8_crash_1.zipx.uu
@@ -0,0 +1,4 @@
+begin 644 test_read_format_zip_ppmd8_crash_1.zipx
+K4$L'"(=02P,$\+N.O&*A>*\+."U``$H`````````@``````#````6(0`````
+`
+end

diff --git a/libarchive/test/test_read_format_zip_ppmd8_crash_2.zipx.uu b/libarchive/test/test_read_format_zip_ppmd8_crash_2.zipx.uu
new file mode 100644 (file)
index 0000000..58de412
--- /dev/null
+++ b/libarchive/test/test_read_format_zip_ppmd8_crash_2.zipx.uu
@@ -0,0 +1,4 @@
+begin 644 test_read_format_zip_ppmd8_crash_2.zipx
+L4$L'"(=02P,$\+N.O&*A>*\+.2U`@$H`````````@``````#````````````
+`
+end