[rar5] Fix infinite loop in header parsing

The change to return `FAILED` instead of `FATAL` for issues that
impact a single entry (but don't necessarily terminate the entire archive)
created a bug in header parsing since `FAILED` wasn't handled in a
header-check loop.

Thank goodness for fuzzing CI!

diff --git a/Makefile.am b/Makefile.am
index a0a5a44a11c1346b41f058aaadf4d813f59f1cb3..b64b085b303afa1b6c3484b26f3cb11a3e547882 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -542,6 +542,7 @@ libarchive_test_SOURCES= \
        libarchive/test/test_read_format_rar_invalid1.c \
        libarchive/test/test_read_format_rar_overflow.c \
        libarchive/test/test_read_format_rar5.c \
+       libarchive/test/test_read_format_rar5_block_hdr_fail_loop.c \
        libarchive/test/test_read_format_rar5_loop_bug.c \
        libarchive/test/test_read_format_raw.c \
        libarchive/test/test_read_format_tar.c \

diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index 6e5656b8fc21c9af6b23ef9340632b090c94b3f2..95eb20c69452e36fc5b87557190ca565c64cc5ed 100644 (file)
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -3887,7 +3887,7 @@ static int do_uncompress_file(struct archive_read* a) {
                 * files). */
                while(1) {
                        ret = process_block(a);
-                       if(ret == ARCHIVE_EOF || ret == ARCHIVE_FATAL)
+                       if(ret != ARCHIVE_OK)
                                return ret;
 
                        if(rar->cstate.last_write_ptr ==

diff --git a/libarchive/test/CMakeLists.txt b/libarchive/test/CMakeLists.txt
index fb9bcdc0d50e8d39ec8a13fc32c817312e07a7f6..345e7f64cbd640f1fc3d17dbcafc330c66981dfd 100644 (file)
--- a/libarchive/test/CMakeLists.txt
+++ b/libarchive/test/CMakeLists.txt
@@ -176,6 +176,7 @@ IF(ENABLE_TEST)
     test_read_format_rar_filter.c
     test_read_format_rar_overflow.c
     test_read_format_rar5.c
+    test_read_format_rar5_block_hdr_fail_loop.c
     test_read_format_rar5_loop_bug.c
     test_read_format_raw.c
     test_read_format_tar.c

diff --git a/libarchive/test/test_read_format_rar5_block_hdr_fail_loop.c b/libarchive/test/test_read_format_rar5_block_hdr_fail_loop.c
new file mode 100644 (file)
index 0000000..e447a2a
--- /dev/null
+++ b/libarchive/test/test_read_format_rar5_block_hdr_fail_loop.c
@@ -0,0 +1,37 @@
+#include "test.h"
+
+/*
+ * Regression test for infinite loop when parse_block_header() returns
+ * ARCHIVE_FAILED inside do_uncompress_file()'s while(1) loop.
+ *
+ * The loop only exited on ARCHIVE_EOF or ARCHIVE_FATAL.  After converting
+ * per-entry block header errors from ARCHIVE_FATAL to ARCHIVE_FAILED, a
+ * malformed block header caused the loop to spin forever.
+ *
+ * Fuzzer-generated reproducer (37 bytes).
+ */
+DEFINE_TEST(test_read_format_rar5_block_hdr_fail_loop)
+{
+       /* RAR5 archive with a malformed compressed block header */
+       static const uint8_t data[] = {
+               0x52,0x61,0x72,0x21,0x1a,0x07,0x01,0x00,
+               0x86,0x7e,0xfa,0xe7,0x03,0x02,0x56,0x20,
+               0x00,0x20,0x15,0xae,0x21,0x00,0x01,0x08,
+               0x00,0x00,0x01,0x00,0x00,0x15,0x00,0xbe,
+               0xc0,0x80,0x00,0xff,0xf4
+       };
+
+       struct archive *a = archive_read_new();
+       assert(a != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_rar5(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_open_memory(a,
+           data, sizeof(data)));
+
+       struct archive_entry *ae;
+       char buf[4096];
+       while (archive_read_next_header(a, &ae) == ARCHIVE_OK)
+               while (archive_read_data(a, buf, sizeof(buf)) > 0)
+                       ;
+
+       assertEqualInt(ARCHIVE_OK, archive_read_free(a));
+}