This commit fixes multiple issues found in the function that parses
extra fields found in the "file"/"service" blocks.
1. In case the file declares just one extra field, which is an
unsupported field, the function returns ARCHIVE_FATAL.
The commit fixes this so this case is allowed, and the unsupported
extra field is skipped. The commit also introduces a test for this
case.
2. Current parsing method of extra fields can report parsing errors in
case the file is malformed. The problem is that next iteration of
parsing, which is meant to process the next extra field (if any),
overwrites the result of the previous iteration, even if previous
iteration has reported parsing error. A successful parse can be
returned in this case, leading to undefined behavior.
This commit changes the behavior to fail the parsing function early.
Also a test file is introduced for this case.
3. In case the file declares only the EX_CRYPT extra field, current
function simply returns ARCHIVE_FATAL, preventing the caller from
setting the proper error string. This results in libarchive returning
an ARCHIVE_FATAL without any error messages set. The PR #2096 (commit
adee36b00) was specifically created to provide error strings in case
EX_CRYPT attribute was encountered, but current behavior contradicts
this case.
The commit changes the behavior so that ARCHIVE_OK is returned by the
extra field parsing function in only EX_CRYPT is encountered, so that
the caller header reading function can properly return ARCHIVE_FATAL
to the caller, at the same time setting a proper error string. A test
file is also provided for this case.
This PR should fix issue #2711.
libarchive/test/test_read_format_rar4_encrypted_filenames.rar.uu \
libarchive/test/test_read_format_rar4_solid_encrypted.rar.uu \
libarchive/test/test_read_format_rar4_solid_encrypted_filenames.rar.uu \
+ libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu \
+ libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu \
+ libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu \
libarchive/test/test_read_format_rar5_encrypted.rar.uu \
libarchive/test/test_read_format_rar5_encrypted_filenames.rar.uu \
libarchive/test/test_read_format_rar5_solid_encrypted.rar.uu \
{
uint64_t extra_field_size;
uint64_t extra_field_id = 0;
- int ret = ARCHIVE_FATAL;
uint64_t var_size;
while(extra_data_size > 0) {
+ /* Make sure we won't fail if the file declares only unsupported
+ attributes. */
+ int ret = ARCHIVE_OK;
+
if(!read_var(a, &extra_field_size, &var_size))
return ARCHIVE_EOF;
if (ARCHIVE_OK != consume(a, extra_field_size)) {
return ARCHIVE_EOF;
}
+
+ /* Don't fail on unsupported attribute -- we've handled it
+ by skipping over it. */
+ ret = ARCHIVE_OK;
+ }
+
+ if (ret != ARCHIVE_OK) {
+ /* Forward any errors signalled by the attribute parsing
+ functions. */
+ return ret;
}
}
- if(ret != ARCHIVE_OK) {
- /* Attribute not implemented. */
- return ret;
+ if (extra_data_size != 0) {
+ /* We didn't skip everything, or we skipped too much; either way,
+ there's an error in this parsing function. */
+
+ archive_set_error(&a->archive, ARCHIVE_ERRNO_PROGRAMMER,
+ "unsupported structure of file header extra data");
+ return ARCHIVE_FATAL;
}
return ARCHIVE_OK;
EPILOGUE();
}
+
+DEFINE_TEST(test_read_format_rar5_only_crypt_exfld)
+{
+ /* GH #2711 */
+
+ char buf[4096];
+ PROLOGUE("test_read_format_rar5_only_crypt_exfld.rar");
+
+ /* The reader should allow iteration through files, but should fail
+ during data extraction. */
+
+ assertA(archive_read_next_header(a, &ae) == ARCHIVE_OK);
+ assertA(archive_read_data(a, buf, sizeof(buf)) == ARCHIVE_FATAL);
+
+ /* The reader should also provide a valid error message. */
+ assertA(archive_error_string(a) != NULL);
+
+ EPILOGUE();
+}
+
+DEFINE_TEST(test_read_format_rar5_only_unsupported_exfld)
+{
+ /* GH #2711 */
+
+ char buf[4096];
+ PROLOGUE("test_read_format_rar5_unsupported_exfld.rar");
+
+ /* The reader should allow iteration through files, and it should
+ succeed with data extraction. */
+
+ assertA(archive_read_next_header(a, &ae) == ARCHIVE_OK);
+
+ /* 48 is the expected number of bytes that should be extracted */
+ assertA(archive_read_data(a, buf, sizeof(buf)) == 48);
+
+ EPILOGUE();
+}
+
+DEFINE_TEST(test_read_format_rar5_invalidhash_and_validhtime_exfld)
+{
+ /* GH #2711 */
+
+ char buf[4096];
+ PROLOGUE("test_read_format_rar5_invalid_hash_valid_htime_exfld.rar");
+
+ /* The reader should report an error when trying to process this data.
+ Returning EOF here means that the reader has failed to identify
+ malformed structure. */
+
+ assertA(archive_read_next_header(a, &ae) < 0);
+ assertA(archive_read_data(a, buf, sizeof(buf)) < 0);
+
+ EPILOGUE();
+}
--- /dev/null
+begin 644 -
+M4F%R(1H'`0`SDK7E"@$%!@`%`0&`@`#^T/5L)`(###$$,>V#`D840I@``0AF
+M:6QE+G1X=`@"OX0]``$"`P(#`&EN=F%L:60@2$%32"!E>'1R82P@86YD(&QA
+>=&5R(&$@=F%L:60@2%1)344@97AT<F$@/=^&`@4$
+`
+end
--- /dev/null
+begin 644 -
+M4F%R(1H'`0`SDK7E"@$%!@`%`0&`@`"!0_"Y/0(#)2X$+NV#`IB>)!,``0AF
+M:6QE+G1X="0!```&``````````````````````````````````````````!R
+M87(U('-T;W)E9"!F:6QE('=I=&@@;VYL>2!A($-265!4(&5X=')A(&9I96QD
+'(#W?A@(%!```
+`
+end
--- /dev/null
+begin 644 -
+M4F%R(1H'`0`SDK7E"@$%!@`%`0&`@`#>[JDS)@(##C`$,.V#`BX6Z[0``0AF
+M:6QE+G1X=`W_____#WA6-!(`````<F%R-2!S=&]R960@9FEL92!W:71H(&%N
+?('5N<W5P<&]R=&5D(&5X=')A(&9I96QD(#W?A@(%!```
+`
+end