From: Grzegorz Antoniak <ga@anadoxin.org>
Date: Sat, 13 Feb 2021 09:13:22 +0000 (+0100)
Subject: RAR5 reader: add more checks for invalid extraction parameters
X-Git-Tag: v3.6.0~7^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F1493%2Fhead;p=thirdparty%2Flibarchive.git

RAR5 reader: add more checks for invalid extraction parameters

Some specially crafted files declare invalid extraction parameters that
can confuse the RAR5 reader.

One of the arguments is the declared window size parameter that the
archive file can declare for each file stored in the archive. Some
crafted files declare window size equal to 0, which is clearly wrong.

This commit adds additional safety checks decreasing the tolerance of
the RAR5 format.

This commit also contains OSSFuzz sample #30459.
---

diff --git a/Makefile.am b/Makefile.am
index 1523c5399..743aaa0db 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -895,6 +895,7 @@ libarchive_test_EXTRA_DIST=\
 	libarchive/test/test_read_format_rar5_block_size_is_too_small.rar.uu \
 	libarchive/test/test_read_format_rar5_decode_number_out_of_bounds_read.rar.uu \
 	libarchive/test/test_read_format_rar5_window_buf_and_size_desync.rar.uu \
+	libarchive/test/test_read_format_rar5_bad_window_sz_in_mltarc_file.rar.uu \
 	libarchive/test/test_read_format_raw.bufr.uu \
 	libarchive/test/test_read_format_raw.data.gz.uu \
 	libarchive/test/test_read_format_raw.data.Z.uu \
diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index 880ba6612..a3cfa72e7 100644
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -3731,6 +3731,16 @@ static int do_uncompress_file(struct archive_read* a) {
 		rar->cstate.initialized = 1;
 	}
 
+	/* Don't allow extraction if window_size is invalid. */
+	if(rar->cstate.window_size == 0) {
+		archive_set_error(&a->archive,
+			ARCHIVE_ERRNO_FILE_FORMAT,
+			"Invalid window size declaration in this file");
+
+		/* This should never happen in valid files. */
+		return ARCHIVE_FATAL;
+	}
+
 	if(rar->cstate.all_filters_applied == 1) {
 		/* We use while(1) here, but standard case allows for just 1
 		 * iteration. The loop will iterate if process_block() didn't
diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c
index 414401a1d..acc905109 100644
--- a/libarchive/test/test_read_format_rar5.c
+++ b/libarchive/test/test_read_format_rar5.c
@@ -1328,3 +1328,22 @@ DEFINE_TEST(test_read_format_rar5_decode_number_out_of_bounds_read)
 
 	EPILOGUE();
 }
+
+DEFINE_TEST(test_read_format_rar5_bad_window_size_in_multiarchive_file)
+{
+	/* oss fuzz 30459 */
+
+	char buf[4096];
+	PROLOGUE("test_read_format_rar5_bad_window_sz_in_mltarc_file.rar");
+
+	/* This file is damaged, so those functions should return failure.
+	 * Additionally, SIGSEGV shouldn't be raised during execution
+	 * of those functions. */
+
+	(void) archive_read_next_header(a, &ae);
+	while(0 < archive_read_data(a, buf, sizeof(buf))) {}
+	(void) archive_read_next_header(a, &ae);
+	while(0 < archive_read_data(a, buf, sizeof(buf))) {}
+
+	EPILOGUE();
+}
\ No newline at end of file
diff --git a/libarchive/test/test_read_format_rar5_bad_window_sz_in_mltarc_file.rar.uu b/libarchive/test/test_read_format_rar5_bad_window_sz_in_mltarc_file.rar.uu
new file mode 100644
index 000000000..7684bc199
--- /dev/null
+++ b/libarchive/test/test_read_format_rar5_bad_window_sz_in_mltarc_file.rar.uu
@@ -0,0 +1,7 @@
+begin 644 test_read_format_rar5_bad_window_size_in_multiarchive_file.rar
+M4F%R(1H'`0`]/-[E`@$`_R`@1#[Z5P("`PL`("`@@"(`"?\@("#___\@("`@
+M("`@("`@("`@4X`J]`,"YR(#$($@("`@``$@("`@@<L0("`@("`@("`@("`@
+M("`@(""LCTJA`P$%`B`@`2!3@"KT`P+G(@,@("`@_P,!!B`@(/___R`@(('+
+5$"`OX2`@[.SL[.S_("`@("`@("`@
+`
+end