diff options
author | Grzegorz Antoniak <ga@anadoxin.org> | 2021-02-12 20:18:31 +0100 |
---|---|---|
committer | Grzegorz Antoniak <ga@anadoxin.org> | 2022-02-06 18:36:23 +0100 |
commit | 17f4e83c0f0fc3bacf4b2bbacb01f987bb5aff5f (patch) | |
tree | 709d45424a7a57dfbf6fff79ea38691ee9f8c6f8 | |
parent | 404873ce40a06f4ff05f76ecbc139a8fabb32d7c (diff) | |
download | libarchive-17f4e83c0f0fc3bacf4b2bbacb01f987bb5aff5f.tar.gz |
RAR5 reader: fix invalid memory access in some files
RAR5 reader uses several variables to manage the window buffer during
extraction: the buffer itself (`window_buf`), the current size of the
window buffer (`window_size`), and a helper variable (`window_mask`)
that is used to constrain read and write offsets to the window buffer.
Some specially crafted files can force the unpacker to update the
`window_mask` variable to a value that is out of sync with current
buffer size. If the `window_mask` will be bigger than the actual buffer
size, then an invalid access operation can happen (SIGSEGV).
This commit ensures that if the `window_size` and `window_mask` will be
changed, the window buffer will be reallocated to the proper size, so no
invalid memory operation should be possible.
This commit contains a test file from OSSFuzz #30442.
-rw-r--r-- | Makefile.am | 1 | ||||
-rw-r--r-- | libarchive/archive_read_support_format_rar5.c | 27 | ||||
-rw-r--r-- | libarchive/test/test_read_format_rar5.c | 17 | ||||
-rw-r--r-- | libarchive/test/test_read_format_rar5_window_buf_and_size_desync.rar.uu | 11 |
4 files changed, 50 insertions, 6 deletions
diff --git a/Makefile.am b/Makefile.am index 7d75ef92..1523c539 100644 --- a/Makefile.am +++ b/Makefile.am @@ -894,6 +894,7 @@ libarchive_test_EXTRA_DIST=\ libarchive/test/test_read_format_rar5_different_winsize_on_merge.rar.uu \ 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_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 a91c73f8..880ba661 100644 --- a/libarchive/archive_read_support_format_rar5.c +++ b/libarchive/archive_read_support_format_rar5.c @@ -1772,14 +1772,29 @@ static int process_head_file(struct archive_read* a, struct rar5* rar, } } - /* If we're currently switching volumes, ignore the new definition of - * window_size. */ - if(rar->cstate.switch_multivolume == 0) { - /* Values up to 64M should fit into ssize_t on every - * architecture. */ - rar->cstate.window_size = (ssize_t) window_size; + if(rar->cstate.window_size < (ssize_t) window_size && + rar->cstate.window_buf) + { + /* If window_buf has been allocated before, reallocate it, so + * that its size will match new window_size. */ + + uint8_t* new_window_buf = + realloc(rar->cstate.window_buf, window_size); + + if(!new_window_buf) { + archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER, + "Not enough memory when trying to realloc the window " + "buffer."); + return ARCHIVE_FATAL; + } + + rar->cstate.window_buf = new_window_buf; } + /* Values up to 64M should fit into ssize_t on every + * architecture. */ + rar->cstate.window_size = (ssize_t) window_size; + if(rar->file.solid > 0 && rar->file.solid_window_size == 0) { /* Solid files have to have the same window_size across whole archive. Remember the window_size parameter diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c index d8b9ff21..414401a1 100644 --- a/libarchive/test/test_read_format_rar5.c +++ b/libarchive/test/test_read_format_rar5.c @@ -1206,6 +1206,23 @@ DEFINE_TEST(test_read_format_rar5_different_window_size) EPILOGUE(); } +DEFINE_TEST(test_read_format_rar5_window_buf_and_size_desync) +{ + /* oss fuzz 30442 */ + + char buf[4096]; + PROLOGUE("test_read_format_rar5_window_buf_and_size_desync.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); + while(0 < archive_read_data(a, buf, 46)) {} + + EPILOGUE(); +} + DEFINE_TEST(test_read_format_rar5_arm_filter_on_window_boundary) { char buf[4096]; diff --git a/libarchive/test/test_read_format_rar5_window_buf_and_size_desync.rar.uu b/libarchive/test/test_read_format_rar5_window_buf_and_size_desync.rar.uu new file mode 100644 index 00000000..9e7d20ff --- /dev/null +++ b/libarchive/test/test_read_format_rar5_window_buf_and_size_desync.rar.uu @@ -0,0 +1,11 @@ +begin 644 test_read_format_rar5_window_buf_and_size_desync.rar +M4F%R(1H'`0`]/-[E`@$`_P$`1#[Z5P("`PL``BXB"?\`!(@B@0`)6.-AF?_1 +M^0DI&0GG(F%R(0<:)`!3@"KT`P+G(@O_X[\``#&``(?!!0$$[:L``$.M*E)A +M<B$`O<\>P0";/P1%``A*2DI*2DYQ<6TN9'%*2DI*2DI*``!D<F--``````"Z +MNC*ZNKJZNFYO=&%I;+JZNKJZNKJZOKJZ.KJZNKJZNKKZU@4%````0$!`0$!` +M0$!`0$!`0$!`0$#_________/T#`0$!`0$!`-UM`0$!`0$!`0$!`0$!`0$!` +M0$!`0'!,J+:O!IZ-WN4'@`!3*F0````````````````````````````````` +M``````````````#T`P)287(A&@<!`%.`*O0#`N<B`_,F@`'[__\``(`4`01S +J'`/H/O\H@?\D`#O9GIZ>GN<B"_]%``(``&1RGIZ>GIZ>8_^>GE/_``!. +` +end |