Merge pull request #2923 from bililateral/pr-7zip-numfiles-oom

7zip: sanity-check FilesInfo NumFiles before allocating entries
(cherry picked from commit 10bd4da47b)

Makefile.am

@@ -882,6 +882,7 @@ libarchive_test_EXTRA_DIST=\
 	libarchive/test/test_read_format_7zip_malformed2.7z.uu \
 	libarchive/test/test_read_format_7zip_malformed3.7z.uu \
 	libarchive/test/test_read_format_7zip_malformed4.7z.uu \
+	libarchive/test/test_read_format_7zip_malformed_numfiles_oom.7z.uu \
 	libarchive/test/test_read_format_7zip_packinfo_digests.7z.uu \
 	libarchive/test/test_read_format_7zip_ppmd.7z.uu \
 	libarchive/test/test_read_format_7zip_sfx_elf.elf.uu \

libarchive/archive_read_support_format_7zip.c

@@ -397,6 +397,27 @@ struct _7zip {
  * the files. */
 #define UMAX_ENTRY	ARCHIVE_LITERAL_ULL(100000000)
 
+/*
+ * Files without unpack streams must be described by the EmptyStream bitmap,
+ * which consumes one bit for every file entry in FilesInfo.
+ */
+static int
+files_info_numfiles_is_sane(const struct _7zip *zip)
+{
+	uint64_t empty_stream_map_bytes;
+
+	if (zip->numFiles > UMAX_ENTRY)
+		return (0);
+	if (zip->numFiles > SIZE_MAX / sizeof(*zip->entries))
+		return (0);
+
+	if (zip->numFiles <= zip->si.ss.unpack_streams)
+		return (1);
+
+	empty_stream_map_bytes = (zip->numFiles + 7) / 8;
+	return (empty_stream_map_bytes <= zip->header_bytes_remaining);
+}
+
 static int	archive_read_format_7zip_has_encrypted_entries(struct archive_read *);
 static int	archive_read_support_format_7zip_capabilities(struct archive_read *a);
 static int	archive_read_format_7zip_bid(struct archive_read *, int);
@@ -2815,15 +2836,7 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
 
 	if (parse_7zip_uint64(a, &(zip->numFiles)) < 0)
 		return (-1);
-	if (UMAX_ENTRY < zip->numFiles)
-		return (-1);
-	/* Empty-file entries (those beyond the known stream count) require a
-	 * kEmptyStream bitmap of ceil(numFiles/8) bytes; reject if that cannot
-	 * fit in the remaining header bytes. Non-empty files need no header
-	 * space here because they map directly to already-parsed streams. */
-	if (zip->numFiles > (uint64_t)zip->si.ss.unpack_streams &&
-	    zip->numFiles - (uint64_t)zip->si.ss.unpack_streams >
-	    8 * zip->header_bytes_remaining)
+	if (!files_info_numfiles_is_sane(zip))
 		return (-1);
 
 	zip->entries = calloc((size_t)zip->numFiles, sizeof(*zip->entries));

libarchive/test/test_read_format_7zip_malformed.c

@@ -93,10 +93,28 @@ test_malformed4(void)
 	assertEqualInt(ARCHIVE_OK, archive_read_free(a));
 }
 
+static void
+test_malformed_numfiles_oom(void)
+{
+	const char *refname = "test_read_format_7zip_malformed_numfiles_oom.7z";
+	struct archive *a;
+	struct archive_entry *ae;
+
+	extract_reference_file(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, archive_read_open_filename(a, refname, 10240));
+	assertEqualIntA(a, ARCHIVE_FATAL, archive_read_next_header(a, &ae));
+	assertEqualInt(ARCHIVE_OK, archive_read_free(a));
+}
+
 DEFINE_TEST(test_read_format_7zip_malformed)
 {
 	test_malformed1();
 	test_malformed2();
 	test_malformed3();
 	test_malformed4();
+	test_malformed_numfiles_oom();
 }

libarchive/test/test_read_format_7zip_malformed_numfiles_oom.7z.uu

@@ -0,0 +1,5 @@
+begin 644 test_read_format_7zip_malformed_numfiles_oom.7z
+M-WJ\KR<<``,%;\HA```````````N`````````,Z^!W,!!>,.`8`/`8`1#0!E
+A`&T`<`!T`'D````4"@$`@-9``*BRG0$5!@$`(("D@0``
+`
+end