diff options
author | Grzegorz Antoniak <ga@anadoxin.org> | 2019-09-27 07:38:58 +0200 |
---|---|---|
committer | Grzegorz Antoniak <ga@anadoxin.org> | 2019-09-27 19:51:32 +0200 |
commit | b09d86145ff02a85b603c54a7e188513d7c2bc4f (patch) | |
tree | dd7f836c41350a3f7e06b95a96203a7741c79cf6 | |
parent | 2f3033ca23f8c21160506c3c7ac8a0df0d3fde42 (diff) | |
download | libarchive-b09d86145ff02a85b603c54a7e188513d7c2bc4f.tar.gz |
RAR5 reader: verify window size for solid files
RAR5 archives can contain files compressed independently of each other,
and files that share a common window buffer, so files which are
compressed using 'solid' method. In the latter case, all files
are required to use the same window buffer, so window size should also
be the same.
OSSFuzz sample #15482 declares a different window size for multiple
solid files. RAR5 reader doesn't reallocate window buffer when
decompressing solid files, so it was possible to perform an
out-of-bounds read by declaring two solid files, where the second solid
file declared the window size parameter that was bigger than window size
used in first solid file.
This commit introduces additional checks to ensure all solid files are
using the same window size.
The commit also adds a test case using OSSFuzz sample #15482 to hunt
down regressions in the future.
Some other test cases had to be adjusted as well, because other OSSFuzz
samples were also declaring different window sizes for solid files. So
this commit has changed the error reporting for those invalid sample files.
-rw-r--r-- | Makefile.am | 1 | ||||
-rw-r--r-- | libarchive/archive_read_support_format_rar5.c | 35 | ||||
-rw-r--r-- | libarchive/test/test_read_format_rar5.c | 75 | ||||
-rw-r--r-- | libarchive/test/test_read_format_rar5_different_solid_window_size.rar.uu | 73 |
4 files changed, 151 insertions, 33 deletions
diff --git a/Makefile.am b/Makefile.am index 20eb5312..03805b4b 100644 --- a/Makefile.am +++ b/Makefile.am @@ -835,6 +835,7 @@ libarchive_test_EXTRA_DIST=\ libarchive/test/test_read_format_rar5_blake2.rar.uu \ libarchive/test/test_read_format_rar5_compressed.rar.uu \ libarchive/test/test_read_format_rar5_different_window_size.rar.uu \ + libarchive/test/test_read_format_rar5_different_solid_window_size.rar.uu \ libarchive/test/test_read_format_rar5_distance_overflow.rar.uu \ libarchive/test/test_read_format_rar5_extra_field_version.rar.uu \ libarchive/test/test_read_format_rar5_fileattr.rar.uu \ diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c index e58cbbf6..e7d7ac42 100644 --- a/libarchive/archive_read_support_format_rar5.c +++ b/libarchive/archive_read_support_format_rar5.c @@ -115,6 +115,8 @@ struct file_header { /* Optional redir fields */ uint64_t redir_type; uint64_t redir_flags; + + ssize_t solid_window_size; /* Used in file format check. */ }; enum EXTRA { @@ -1665,6 +1667,17 @@ static int process_head_file(struct archive_read* a, struct rar5* rar, g_unpack_window_size << ((compression_info >> 10) & 15); rar->cstate.method = c_method; rar->cstate.version = c_version + 50; + rar->file.solid = (compression_info & SOLID) > 0; + + /* Archives which declare solid files without initializing the window + * buffer first are invalid. */ + + if(rar->file.solid > 0 && rar->cstate.window_buf == NULL) { + archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, + "Declared solid file, but no window buffer " + "initialized yet."); + return ARCHIVE_FATAL; + } /* Check if window_size is a sane value. Also, if the file is not * declared as a directory, disallow window_size == 0. */ @@ -1676,12 +1689,32 @@ static int process_head_file(struct archive_read* a, struct rar5* rar, return ARCHIVE_FATAL; } + if(rar->file.solid > 0) { + /* Re-check if current window size is the same as previous + * window size (for solid files only). */ + if(rar->file.solid_window_size > 0 && + rar->file.solid_window_size != (ssize_t) window_size) + { + archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, + "Window size for this solid file doesn't match " + "the window size used in previous solid file. "); + return ARCHIVE_FATAL; + } + } + /* 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 + for first solid file found. */ + rar->file.solid_window_size = rar->cstate.window_size; + } + init_window_mask(rar); - rar->file.solid = (compression_info & SOLID) > 0; rar->file.service = 0; if(!read_var_sized(a, &host_os, NULL)) diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c index b4ef29e4..f44b55ae 100644 --- a/libarchive/test/test_read_format_rar5.c +++ b/libarchive/test/test_read_format_rar5.c @@ -969,13 +969,12 @@ DEFINE_TEST(test_read_format_rar5_readtables_overflow) PROLOGUE("test_read_format_rar5_readtables_overflow.rar"); - assertA(0 == archive_read_next_header(a, &ae)); /* This archive is invalid. However, processing it shouldn't cause any * buffer overflow errors during reading rar5 tables. */ - assertA(archive_read_data(a, buf, sizeof(buf)) <= 0); - /* This test only cares about not returning success here. */ - assertA(ARCHIVE_OK != archive_read_next_header(a, &ae)); + (void) archive_read_next_header(a, &ae); + (void) archive_read_data(a, buf, sizeof(buf)); + (void) archive_read_next_header(a, &ae); EPILOGUE(); } @@ -986,13 +985,12 @@ DEFINE_TEST(test_read_format_rar5_leftshift1) PROLOGUE("test_read_format_rar5_leftshift1.rar"); - assertA(0 == archive_read_next_header(a, &ae)); /* This archive is invalid. However, processing it shouldn't cause any * errors related to undefined operations when using -fsanitize. */ - assertA(archive_read_data(a, buf, sizeof(buf)) <= 0); - /* This test only cares about not returning success here. */ - assertA(ARCHIVE_OK != archive_read_next_header(a, &ae)); + (void) archive_read_next_header(a, &ae); + (void) archive_read_data(a, buf, sizeof(buf)); + (void) archive_read_next_header(a, &ae); EPILOGUE(); } @@ -1003,14 +1001,12 @@ DEFINE_TEST(test_read_format_rar5_leftshift2) PROLOGUE("test_read_format_rar5_leftshift2.rar"); - assertA(0 == archive_read_next_header(a, &ae)); - /* This archive is invalid. However, processing it shouldn't cause any * errors related to undefined operations when using -fsanitize. */ - assertA(archive_read_data(a, buf, sizeof(buf)) <= 0); - /* This test only cares about not returning success here. */ - assertA(ARCHIVE_OK != archive_read_next_header(a, &ae)); + (void) archive_read_next_header(a, &ae); + (void) archive_read_data(a, buf, sizeof(buf)); + (void) archive_read_next_header(a, &ae); EPILOGUE(); } @@ -1021,14 +1017,12 @@ DEFINE_TEST(test_read_format_rar5_truncated_huff) PROLOGUE("test_read_format_rar5_truncated_huff.rar"); - assertA(0 == archive_read_next_header(a, &ae)); - /* This archive is invalid. However, processing it shouldn't cause any * errors related to undefined operations when using -fsanitize. */ - assertA(archive_read_data(a, buf, sizeof(buf)) <= 0); - /* This test only cares about not returning success here. */ - assertA(ARCHIVE_OK != archive_read_next_header(a, &ae)); + (void) archive_read_next_header(a, &ae); + (void) archive_read_data(a, buf, sizeof(buf)); + (void) archive_read_next_header(a, &ae); EPILOGUE(); } @@ -1058,14 +1052,12 @@ DEFINE_TEST(test_read_format_rar5_distance_overflow) PROLOGUE("test_read_format_rar5_distance_overflow.rar"); - assertA(0 == archive_read_next_header(a, &ae)); - /* This archive is invalid. However, processing it shouldn't cause any * errors related to variable overflows when using -fsanitize. */ - assertA(archive_read_data(a, buf, sizeof(buf)) <= 0); - /* This test only cares about not returning success here. */ - assertA(ARCHIVE_OK != archive_read_next_header(a, &ae)); + (void) archive_read_next_header(a, &ae); + (void) archive_read_data(a, buf, sizeof(buf)); + (void) archive_read_next_header(a, &ae); EPILOGUE(); } @@ -1076,14 +1068,12 @@ DEFINE_TEST(test_read_format_rar5_nonempty_dir_stream) PROLOGUE("test_read_format_rar5_nonempty_dir_stream.rar"); - assertA(0 == archive_read_next_header(a, &ae)); - /* This archive is invalid. However, processing it shouldn't cause any * errors related to buffer overflows when using -fsanitize. */ - assertA(archive_read_data(a, buf, sizeof(buf)) <= 0); - /* This test only cares about not returning success here. */ - assertA(ARCHIVE_OK != archive_read_next_header(a, &ae)); + (void) archive_read_next_header(a, &ae); + (void) archive_read_data(a, buf, sizeof(buf)); + (void) archive_read_next_header(a, &ae); EPILOGUE(); } @@ -1205,13 +1195,13 @@ DEFINE_TEST(test_read_format_rar5_different_window_size) * errors during processing. */ (void) archive_read_next_header(a, &ae); - while(0 != archive_read_data(a, buf, sizeof(buf))) {} + while(0 < archive_read_data(a, buf, sizeof(buf))) {} (void) archive_read_next_header(a, &ae); - while(0 != archive_read_data(a, buf, sizeof(buf))) {} + while(0 < archive_read_data(a, buf, sizeof(buf))) {} (void) archive_read_next_header(a, &ae); - while(0 != archive_read_data(a, buf, sizeof(buf))) {} + while(0 < archive_read_data(a, buf, sizeof(buf))) {} EPILOGUE(); } @@ -1226,7 +1216,28 @@ DEFINE_TEST(test_read_format_rar5_arm_filter_on_window_boundary) * errors during processing. */ (void) archive_read_next_header(a, &ae); - while(0 != archive_read_data(a, buf, sizeof(buf))) {} + while(0 < archive_read_data(a, buf, sizeof(buf))) {} + + EPILOGUE(); +} + +DEFINE_TEST(test_read_format_rar5_different_solid_window_size) +{ + char buf[4096]; + PROLOGUE("test_read_format_rar5_different_solid_window_size.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, sizeof(buf))) {} + + (void) archive_read_next_header(a, &ae); + while(0 < archive_read_data(a, buf, sizeof(buf))) {} + + (void) archive_read_next_header(a, &ae); + while(0 < archive_read_data(a, buf, sizeof(buf))) {} EPILOGUE(); } diff --git a/libarchive/test/test_read_format_rar5_different_solid_window_size.rar.uu b/libarchive/test/test_read_format_rar5_different_solid_window_size.rar.uu new file mode 100644 index 00000000..b131e6f0 --- /dev/null +++ b/libarchive/test/test_read_format_rar5_different_solid_window_size.rar.uu @@ -0,0 +1,73 @@ +begin 644 test_read_format_rar5_different_solid_window_size.rar +M4F%R(1H'`0"-[P+2``'#M#P\7P$'`0"-[P+2``7#````1H68F`#___\````` +M```0^OKZ^OJ%F)B8F)A)`)@"F-(%87)4`,.T/#Q?`0<!`(WO`M(`4F%R(1H' +M`0"-[P+2``+'#PD`<@$A!QH:(](M``$:(](M``(:!P$`C>\"T@`"QP\`"7(A +MFC$!$AHCTBT``B@A4F%2(1H'&.D````!`(WO`M(`!<-%````1A?'#P`)<B$: +M!P$:T@7"F!=A_________P$$____,/__F!=A)F%R*%)8(W=T4F%R(1H'`0"- +M[P+2``+'#PD`<@$A!QH:(](M``$:(](M``(:!P$`C>\"T@`"QP\`"7(AFC$! +M$AHCTBT``B@A4F%2(1H'&.D````!`(WO`M(`!<-%````1A?'#P`)<B$:!P$: +MT@7"F!=A_________P$$____,/__F!=A)B8(<@IE`.\*"7(````````````` +M=O____________________\`!<-%````1A?'#_\P__\$(DT89%`(`0(`@$U3 +M0T8`````Y!]W__\;-A8T``````"!`$#U`@`!`&@````!]9^?```5`"T-```` +M`6QT(%P*"75I9%P*"75I9%P*"75I9%P*"75I9%P*"75I9%P*"7456BUL:#4M +M#0````%L<0!SI/\````!]9^?```5`"T-`````6P56BUL:#4M#0````%L<0!S +MI/\````!]9^?```5`"T-``#_``#J`"L-"E=!`````````````````````!$` +M`!04%!04%!06`O__%*"@H*"@H*"@_PIW\M$"+@L*+@I&"@HV"BX*"BXH"F0* +M+@H*+@HNYWJ""BX*"C?^___L+H`NCPHNMX*"@H*"@H*"@H*"@H*"@H*"@H*" +M@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*" +M"@HV"BX*"BXH"F0*+@H*+@HNYWJ""BX*"C?^___L+H`N"BZW"F?_&_9<9``` +M@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"@H*"`/\G`)$` +M@`#L`!OV7&0```%L%5HM;&@U+0T````!;'$`<Z3_`````71Y<&4];`HN"BX* +M+@HN"BX*+FIJ:FIJ:@HN,#@T279R(&1E=FEC93US=B]S964];`HN"BX*+G0@ +M9'!Y93UL"BX*+@HN"BX*+@HN:FIJ:E)#)R\V,#@U+W-E="!\>7!E/6P*+@HN +M"BYT('1Y<&4];`HN,"X*+@HN="!T>7!E/2X*+@HN"BYJ:FHP+W-E93UL"BX* +M+@HN="!T>7!E+W-E="!T>7!E/6P*+@HN"BYT+G0@='G^6_]P9</#P\/#P\/# +MP\/#P\/#P\/#P\/#P\/"P\/#P\/#+W-E="!D979I8V4]8G-D<V5T(&1E=FEC +M93UB<V1O<RPM9"PN9"PN"BX*<R=D+2PL+F0L+@HN"B=S+"UD+"X*+@HN"BX* +M+@HN"BL*70I="ET*70I="ET*70I="ET*70I="ET*70I="ET*70I="ET*70I= +M"B%="ET*70I="ET*70I="ET*70I="ET*70I="@I="ET*70I="ET*70I="ET* +M70I="ET*70I="ET*70I="ET*70I="ET*70H*70I="ET*70I="ET*70I="ET* +M70I="ET*70I="ET*70I="ET*70I="ET*70I="ET*70I="ET*70I="ET*70I= +M"ET*70I="ET*75T*70I="ET*70I="ET*9"PN"BX*+@HN"BX*"BX*+BYS9&7_ +M]P```````,/#P\/#P\/#P\/#P\/#P\/#P\/#P\/#P\,``"D(-```!0`!``$` +M@A8!!0`````````````````!```(\/#P\/!A\/#P\/#P\.+P\/#P\/#P,O#P +M\%!+`P1.`%IB#B@!`/P```#M@!4```###P\/#V?QZ@\/#P\/#P\/#P\/#P\/ +M#P\/#P\/#P\/#P\/#P\/#U37[OL```!02R&PHU:*0,/#)\+#S</#0\/#PP\` +M#P\!]9^?```5`"T-`````6P56BUL:#4M#0````%L<0!SI/\````!]9^?```5 +M`"T-`````6P56BUL:#4M#0````%L<0!SI/\````!]9^?```5`"T-`````6P5 +M6BUL:#4M#0````%L<0!SI/\````!]9^?```5`"T-`````6P56BUL:#4M#0`` +M``%L<0!SI/\````!]9^?```%`"T-`````8Z.CHXICHZ.K0'H```````````` +M`````/]L%5HM;&@U+0T````!;'$`<Z3_`````?6?GP``%0`M#0````%L%5HM +M;&@U+0T````!;'$`<Z3_`````?6?GP``%0`M#0````%L%5HM;&@U+0T````` +M;'$`<Z3_`````?6?GP``%0`M#0````%L%5HM;&@U+=(``0!R(8WO`M(`M`%? +MC>\"T@`"TGX!M,-'4CQ2:7`)20`&`````````(WO`M(``0!R(8WO`M(``0!R +M(8WO`M(``0!R(8WO`M(`M`%?C>\"T@`"TGX!M,-'4CQ:!@!I`'`)20`````` +M`(WO`M(``0!R(8WO`M(``0!R(8WO`M(``0!R(8WO`M(`M`%?C>^NT@`"TGX! +MM%)A<B$:!P$`C>\"T@`"PP<<@`<`_O__T?___^@@JP#_Q00``B$<`0(`#@`` +M`0``_@C2(`$>____"```_?__$`#_W5A04)#_!&R&;;%DS,RWL0!)`(/__P#_ +M`/\`^?__\&3/)#)(L0!)`#J#+O_______REH%PHHV#!="0"N)$%SF_J;$___ +M_YP#%9,I````_________Q/_____________________________________ +M_RC_____`P````````#_____*/]!04%!0=U891/_9?]0/2[_______\I:!<* +M*-@P70D`KB1!<YOZFQ/___^<_^5L*0```/________\3________________ +M______________________\H____/0T.#0I5"E!!W5AE$_]E_U`]/0T.#0I5 +M"E#;V^+;V]O;V____________W________________________________W_ +ME/\56BUL:#4M#0````%L<0!SI/\````!]9^?```5`"T-`````6P56BUL:#4M +M#0````%L<0!SI/\````!]9^?```5`"T-`````6P56BUL:#4M#0````%L<0!S +MI/\````!]9^?```5`"T-`````6P56BUL:#4M#0````%L<0!SI/\````!]9^? +M```5`"T-`````6P56BUL:#4M#0````%L<0!SI/\````!]9^?```5`"T-```` +M`6P56BUL:#4M#0````%L<0!SI/\````!]9^?```5`"T-`````6P56BUL:#4M +M#0````%L<0!SI/\````!]9^?8C,*-`HN"EQ<7%Q<7%Q<7%Q<7%Q<7%Q<7%Q< +M7`3_________`F@`!+_)_P\LL`#__V%R(1H'``1G=$Q24``0````(`$````` +M```,J7\`+@TU'`#]_P`!````````!EQ<8C<*-`HN"EQ<7%Q<7%Q<7%Q<7%Q< +M7%Q<7%Q<7`3__RTQ#0I#;U!+`P31!-'1!-$$*%+KZB$6____[``````````` +M-WJ\KR<<\9WSM=;L```````````V``````#___^"@H(!!0("`@("`@("`@(" +M`@(/#P\/#P\/#P\/#U0ZS<@`_\/_/\/#P\/#P\/#P\/#P\/#P\/#P\/#P\/# +MP\/#P\/#P\/#P\/#P\/#P\/#P\/#P\/#P\/#P\/'P\/#PT```(F?GY_RGP&? +MWY^?]4```(F?GY_RGP&?WY^?]4```(DO<V5T(&9L86=S/0HN"BX*+@HN"BX* +M+@HN"BXN"BX*+@HN"BX*+@HN"BX*"BX*+@HN"BX*)@HN"BX*+@HN"BX*+@HN +M"BX*+@HN"BX*+@HN"BX*+@HN"BX*+@HN"BX*+@`*+@HN"BY^+@HN"BX*+@HN +M"BX*+@HN"@HN"BX*+@HN"BX*+@HN"BX*+@HA&@<`!&=T3%)0`!`````@4$L# +M!#$`</^I?P`N#34<`/W_``$7%@I<7"Y<7%RT`5^-[P+2```$O\FTPT=2/%II +9<`E)``8`````````C>\"T@`````````````` +` +end |