RAR5 reader: clear 'data ready' cache on window buffer reallocs (#2265)

The RAR5 reader is using a small stack of cached pointers to submit the
rendered data to the caller. In malformed files it's possible for this
pointer cache to be desynchronized with the memory buffer those pointers
are pointing to, making libarchive crash on invalid memory access.

OSS-Fuzz Issue: 70024

diff --git a/Makefile.am b/Makefile.am
index 532b367c23cdd30f0b2253781d46c14d5de16611..c3adef95fc52ad0718f0ab6d65976477b176ee07 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -925,6 +925,7 @@ libarchive_test_EXTRA_DIST=\
        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_rar5_data_ready_pointer_leak.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 8907fdb2fce784f697ec9aaf660dac5feec5d1c3..d23ef31844bed377dcc30d62073ef31ddfdd1e13 100644 (file)
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -361,6 +361,7 @@ static int verify_global_checksums(struct archive_read* a);
 static int rar5_read_data_skip(struct archive_read *a);
 static int push_data_ready(struct archive_read* a, struct rar5* rar,
        const uint8_t* buf, size_t size, int64_t offset);
+static void clear_data_ready_stack(struct rar5* rar);
 
 /* CDE_xxx = Circular Double Ended (Queue) return values. */
 enum CDE_RETURN_VALUES {
@@ -652,6 +653,7 @@ static int run_filter(struct archive_read* a, struct filter_info* flt) {
        int ret;
        struct rar5* rar = get_context(a);
 
+       clear_data_ready_stack(rar);
        free(rar->cstate.filtered_buf);
 
        rar->cstate.filtered_buf = malloc(flt->block_length);
@@ -1780,6 +1782,13 @@ static int process_head_file(struct archive_read* a, struct rar5* rar,
        if(rar->cstate.window_size < (ssize_t) window_size &&
            rar->cstate.window_buf)
        {
+               /* The `data_ready` stack contains pointers to the `window_buf` or
+                * `filtered_buf` buffers.  Since we're about to reallocate the first
+                * buffer, some of those pointers could become invalid. Therefore, we
+                * need to dispose of all entries from the stack before attempting the
+                * realloc. */
+               clear_data_ready_stack(rar);
+
                /* If window_buf has been allocated before, reallocate it, so
                 * that its size will match new window_size. */
 
@@ -2455,6 +2464,8 @@ static void init_unpack(struct rar5* rar) {
                rar->cstate.filtered_buf = NULL;
        }
 
+       clear_data_ready_stack(rar);
+
        rar->cstate.write_ptr = 0;
        rar->cstate.last_write_ptr = 0;
 
@@ -3629,6 +3640,10 @@ static int use_data(struct rar5* rar, const void** buf, size_t* size,
        return ARCHIVE_RETRY;
 }
 
+static void clear_data_ready_stack(struct rar5* rar) {
+       memset(&rar->cstate.dready, 0, sizeof(rar->cstate.dready));
+}
+
 /* Pushes the `buf`, `size` and `offset` arguments to the rar->cstate.dready
  * FIFO stack. Those values will be popped from this stack by the `use_data`
  * function. */
@@ -4187,6 +4202,7 @@ static int rar5_cleanup(struct archive_read *a) {
 
        free(rar->cstate.window_buf);
        free(rar->cstate.filtered_buf);
+       clear_data_ready_stack(rar);
 
        free(rar->vol.push_buf);
 

diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c
index e51ed53d0cc6c642e690497094cab21bf1a9b070..594f032fc07e405ed2b6643eeb842354ca988a7e 100644 (file)
--- a/libarchive/test/test_read_format_rar5.c
+++ b/libarchive/test/test_read_format_rar5.c
@@ -1344,7 +1344,7 @@ DEFINE_TEST(test_read_format_rar5_sfx)
 
        assertA(size == archive_read_data(a, buff, size));
        assertEqualMem(buff, test_txt, size);
-       
+
        EPILOGUE();
 }
 
@@ -1406,3 +1406,26 @@ DEFINE_TEST(test_read_format_rar5_read_data_block_uninitialized_offset)
 
        EPILOGUE();
 }
+
+DEFINE_TEST(test_read_format_rar5_data_ready_pointer_leak)
+{
+       /* oss fuzz 70024 */
+
+       char buf[4096];
+       PROLOGUE("test_read_format_rar5_data_ready_pointer_leak.rar");
+
+       /* Return codes of those calls are ignored, because this sample file
+        * is invalid. However, the unpacker shouldn't produce any SIGSEGV
+        * errors during processing. */
+
+       (void) archive_read_next_header(a, &ae);
+       (void) archive_read_data(a, buf, sizeof(buf));
+       (void) archive_read_next_header(a, &ae);
+       (void) archive_read_data(a, buf, sizeof(buf));
+       (void) archive_read_data(a, buf, sizeof(buf));
+       (void) archive_read_next_header(a, &ae);
+       /* This call shouldn't produce SIGSEGV. */
+       (void) archive_read_data(a, buf, sizeof(buf));
+
+       EPILOGUE();
+}

diff --git a/libarchive/test/test_read_format_rar5_data_ready_pointer_leak.rar.uu b/libarchive/test/test_read_format_rar5_data_ready_pointer_leak.rar.uu
new file mode 100644 (file)
index 0000000..8c8c907
--- /dev/null
+++ b/libarchive/test/test_read_format_rar5_data_ready_pointer_leak.rar.uu
@@ -0,0 +1,28 @@
+begin 644 test_read_format_rar5_data_ready_pointer_leak.rar.uu
+M4F%R(1H'`0`]/-[E`@$`_R`@1#[Z5P("`P,`(/__(""`((``"2`@("`@_R`@
+M("`@(%.`*O0#`N<B`A"`("#_((`+`00@("`@(""!)/\@("`@("`@(/\@("`@
+M^I@R<00"WN<-\@$@_R`@(``C("`@("`@("`@("`@("`@("`@("`@("`@("#_
+M____("`@(/___R#_("`@("`@("#___\@_____R`@("`@("`@(/__________
+M____(/__(/\@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@________
+M_________R`@____(/____\@("`@("`@("#______________R#___\@("`@
+M("`@("`@("`@("`@("`@("`@(/\@("`@("`@("`@("`@("`@("`@("`@("`@
+M("`@("`@("`@("`@("`@("`@("`@("`@_________R`@("`@_R`@("`@("`@
+M("`@("`@(/__("`@("`@("`@("`@("`@(%.`*O0#`N<B`A"`_R`@_R#_"P$$
+M("`@("`@@23_("`@_R`@("#_("`@(/J8,G$$`M[G#?(!(/\@("``(R`@("`@
+M("`@("`@("`@("`@("`@("`@("#_("`@("`@("#_("`@("`@("`@("`@("`@
+M("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("#_
+M("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@____("`@("`@
+M("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@
+M("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@
+M("`@("`@("`@("`@("`@("`@_R`@("`@("`@("`@("`@("`@("`@("`@("`@
+M("!3@"KT`P+G(B`0@/\@(/\@_PL!_R"!)/\@("`@("`@(/\@("`@^I@R<00"
+MWN<-\@$@_R`@(``C("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@
+M("`@("#______________R`@("`@("`@("`@("`@("`@("`@("`@("`@("`@
+M("`@("`@("`@("`@("`@("`@("`@(/\@(/_______R`@("`@("`@("`@("`@
+M("`@("`@("`@_________R`@("`@("`@("`@("`@("`@(/________\@("`@
+M("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@(/\@
+M("`@("`@(/\@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@("`@
+M("`@("`@("`@("`@("`@(/\@($0^^E<"`@,#("`@(/\@@``)("`@("`@("`@
+)("`@("`@("`@
+`
+end