From: Grzegorz Antoniak Date: Sun, 7 Jul 2024 17:46:42 +0000 (+0200) Subject: RAR5 reader: clear 'data ready' cache on window buffer reallocs (#2265) X-Git-Tag: v3.7.5~17 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6db1836fa6bf8501f907170f6a60da5c5e502913;p=thirdparty%2Flibarchive.git 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 532b367c2..c3adef95f 100644 --- 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 8907fdb2f..d23ef3184 100644 --- 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 e51ed53d0..594f032fc 100644 --- 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 index 000000000..8c8c90778 --- /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