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 1523c5399ecd3fa9ae5d32304d9f4a3dbd3fd2e0..743aaa0db05c5b5a526e05b95d11003d6acc1037 100644 (file)
--- 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 880ba66123d852f071d851b795471c60c26fa28e..a3cfa72e77c8c31bdcedecbaabb36fc1f3257ec3 100644 (file)
--- 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 414401a1ddf3187c88a42ed69600f6b45cdc4a1b..acc90510946bd25e2dc233413825ae36817a1149 100644 (file)
--- 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 (file)
index 0000000..7684bc1
--- /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