]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
RAR5 reader: fix multiple issues in extra field parsing function 2713/head
authorGrzegorz Antoniak <ga@anadoxin.org>
Fri, 1 Aug 2025 20:02:30 +0000 (22:02 +0200)
committerGrzegorz Antoniak <ga@anadoxin.org>
Sun, 3 Aug 2025 13:28:58 +0000 (15:28 +0200)
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.

Makefile.am
libarchive/archive_read_support_format_rar5.c
libarchive/test/test_read_format_rar5.c
libarchive/test/test_read_format_rar5_invalid_hash_valid_htime_exfld.rar.uu [new file with mode: 0644]
libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu [new file with mode: 0644]
libarchive/test/test_read_format_rar5_unsupported_exfld.rar.uu [new file with mode: 0644]

index 9dd37bb30054e0243baa2ebb58b74cc7124c96f1..9d8873d1d987715440fd943fc11c2b08f67fcc28 100644 (file)
@@ -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 \
index 48dde0c2e8140974bf021c35968096b056691a12..f2af82cfadd31b84fc328aef480b0d609c3c1145 100644 (file)
@@ -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;
index fd233277bc1b3f3220871d0e36090088797883f8..202b66f680082eab1adc729e906b585df04b2d64 100644 (file)
@@ -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
@@ -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
@@ -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
@@ -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