From: Grzegorz Antoniak <ga@anadoxin.org>
Date: Fri, 1 Mar 2019 07:32:00 +0000 (+0100)
Subject: ZIP reader: fix a crash in PPMd8 decompressor.
X-Git-Tag: v3.4.0~113^2~1
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b052bdcc299ad4bf812c89c266c9fb909715ebf;p=thirdparty%2Flibarchive.git

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 b91b66f6b..723ea1e04 100644
--- 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 d152a19b4..bc04933c3 100644
--- 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
index 000000000..fb6050fc0
--- /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
index 000000000..58de41258
--- /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