Merge pull request #2713 from antekone/bug/GH-2711/crash-when-rr/1

RAR5 reader: fix multiple issues in extra field parsing function

(cherry picked from commit 93f9e93d73caa0d0a06e3d32cb7483a11afc41e9)

diff --git a/Makefile.am b/Makefile.am
index 9dd37bb30054e0243baa2ebb58b74cc7124c96f1..9d8873d1d987715440fd943fc11c2b08f67fcc28 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -921,6 +921,9 @@ libarchive_test_EXTRA_DIST=\
        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 \

diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index 48dde0c2e8140974bf021c35968096b056691a12..f2af82cfadd31b84fc328aef480b0d609c3c1145 100644 (file)
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -1619,10 +1619,13 @@ static int process_head_file_extra(struct archive_read* a,
 {
        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;
 
@@ -1675,12 +1678,26 @@ static int process_head_file_extra(struct archive_read* a,
                                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;

diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c
index fd233277bc1b3f3220871d0e36090088797883f8..202b66f680082eab1adc729e906b585df04b2d64 100644 (file)
--- a/libarchive/test/test_read_format_rar5.c
+++ b/libarchive/test/test_read_format_rar5.c
@@ -1428,3 +1428,57 @@ DEFINE_TEST(test_read_format_rar5_data_ready_pointer_leak)
 
        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();
+}

diff --git a/libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu b/libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu
new file mode 100644 (file)
index 0000000..399acd8
--- /dev/null
+++ b/libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu
@@ -0,0 +1,6 @@
+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

diff --git a/libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu b/libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu
new file mode 100644 (file)
index 0000000..4f9faf3
--- /dev/null
+++ b/libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu
@@ -0,0 +1,7 @@
+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

diff --git a/libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu b/libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu
new file mode 100644 (file)
index 0000000..16b456b
--- /dev/null
+++ b/libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu
@@ -0,0 +1,6 @@
+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