summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Antoniak <ga@anadoxin.org>2021-02-12 20:18:31 +0100
committerGrzegorz Antoniak <ga@anadoxin.org>2022-02-06 18:36:23 +0100
commit17f4e83c0f0fc3bacf4b2bbacb01f987bb5aff5f (patch)
tree709d45424a7a57dfbf6fff79ea38691ee9f8c6f8
parent404873ce40a06f4ff05f76ecbc139a8fabb32d7c (diff)
downloadlibarchive-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.am1
-rw-r--r--libarchive/archive_read_support_format_rar5.c27
-rw-r--r--libarchive/test/test_read_format_rar5.c17
-rw-r--r--libarchive/test/test_read_format_rar5_window_buf_and_size_desync.rar.uu11
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