]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
rar5: consume unconsumed block bytes before ARCHIVE_RETRY 3091/head
authoryPin9 <alen0421@gmail.com>
Sat, 30 May 2026 12:19:25 +0000 (20:19 +0800)
committeryPin9 <alen0421@gmail.com>
Sat, 30 May 2026 12:19:25 +0000 (20:19 +0800)
process_base_block() returned ARCHIVE_RETRY for HEAD_MAIN (and
HFL_SKIP_IF_UNKNOWN) blocks without consuming the body bytes the
sub-parser did not read. rar5_read_header() then re-parsed the same
region, turning an O(1) skip into O(N) and letting a crafted RAR5 file
stall the reader (GHSA-9h2c-464f-j3hj).

Record the block body start and skip any unconsumed bytes through a
small helper rar5_skip_remaining_block() before returning ARCHIVE_RETRY.

Add two regression tests derived from test_read_format_rar5_stored, each
with extra unread bytes appended to a no-data block's body (HEAD_MAIN and
an unknown HFL_SKIP_IF_UNKNOWN block); both fail on master and pass with
the fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Makefile.am
libarchive/archive_read_support_format_rar5.c
libarchive/test/test_read_format_rar5.c
libarchive/test/test_read_format_rar5_main_block_extra_bytes.rar.uu [new file with mode: 0644]
libarchive/test/test_read_format_rar5_skip_block_extra_bytes.rar.uu [new file with mode: 0644]

index 9d9a637a9797ec41a86ceeea2b89150aa63d3102..8ba267d9027ee0db873206b6bcd436bf35fda2e5 100644 (file)
@@ -981,7 +981,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_main_block_extra_bytes.rar.uu \
        libarchive/test/test_read_format_rar5_only_crypt_exfld.rar.uu \
+       libarchive/test/test_read_format_rar5_skip_block_extra_bytes.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 \
index 1f8f8f435962aabfa72180fd869c450b4125ea36..b5379408e3150dd2e75fb28f4abeb727a992dab9 100644 (file)
@@ -2274,6 +2274,33 @@ static int scan_for_signature(struct archive_read* a);
  * <FILE> block.
  */
 
+/*
+ * A header that carries no file data (HEAD_MAIN, or an unknown block
+ * flagged HFL_SKIP_IF_UNKNOWN) may leave bytes in its body that the
+ * sub-parser did not read. Skip them before returning ARCHIVE_RETRY,
+ * otherwise rar5_read_header() re-parses the same block region O(N)
+ * times instead of O(1), letting a crafted RAR5 file stall the reader
+ * (GHSA-9h2c-464f-j3hj).
+ *
+ * Safe because read_ahead(a, hdr_size, &p) pre-loaded the whole block
+ * into one contiguous buffer with no compaction until we return, so
+ * body_start stays valid and (cur - body_start) is the exact number of
+ * body bytes consumed so far.
+ */
+static void
+rar5_skip_remaining_block(struct archive_read* a,
+    const uint8_t* body_start, size_t raw_hdr_size)
+{
+       const uint8_t* cur;
+
+       if(read_ahead(a, 1, &cur)) {
+               size_t body_used = (size_t)(cur - body_start);
+
+               if(body_used < raw_hdr_size)
+                       (void)consume(a, raw_hdr_size - body_used);
+       }
+}
+
 static int process_base_block(struct archive_read* a,
     struct archive_entry* entry)
 {
@@ -2285,6 +2312,7 @@ static int process_base_block(struct archive_read* a,
        size_t header_id = 0;
        size_t header_flags = 0;
        const uint8_t* p;
+       const uint8_t* body_start;
        int ret;
 
        enum HEADER_TYPE {
@@ -2346,6 +2374,10 @@ static int process_base_block(struct archive_read* a,
 #endif
        }
 
+       /* Remember the first byte of the block body so we can later skip
+        * any bytes the sub-parser leaves unconsumed. */
+       body_start = p + hdr_size_len;
+
        /* If the checksum is OK, we proceed with parsing. */
        if(ARCHIVE_OK != consume(a, hdr_size_len)) {
                return ARCHIVE_EOF;
@@ -2371,8 +2403,11 @@ static int process_base_block(struct archive_read* a,
                        /* Main header doesn't have any files in it, so it's
                         * pointless to return to the caller. Retry to next
                         * header, which should be HEAD_FILE/HEAD_SERVICE. */
-                       if(ret == ARCHIVE_OK)
+                       if(ret == ARCHIVE_OK) {
+                               rar5_skip_remaining_block(a, body_start,
+                                   raw_hdr_size);
                                return ARCHIVE_RETRY;
+                       }
 
                        return ret;
                case HEAD_SERVICE:
@@ -2432,6 +2467,8 @@ static int process_base_block(struct archive_read* a,
                                /* If the block is marked as 'skip if unknown',
                                 * do as the flag says: skip the block
                                 * instead on failing on it. */
+                               rar5_skip_remaining_block(a, body_start,
+                                   raw_hdr_size);
                                return ARCHIVE_RETRY;
                        }
        }
index 4ab5690c46ac39d8e70b0c8440624cd78e9a785e..29cd17767dc42ae0353ffd0223f3ecc59fbd2274 100644 (file)
@@ -1494,3 +1494,52 @@ DEFINE_TEST(test_read_format_rar5_invalidhash_and_validhtime_exfld)
 
        EPILOGUE();
 }
+
+/*
+ * Regression tests for the RAR5 base-block parser leaving unconsumed body
+ * bytes before returning ARCHIVE_RETRY (GHSA-9h2c-464f-j3hj). Each archive is
+ * the test_read_format_rar5_stored archive with extra, unread bytes appended
+ * to a no-data block's body. Before the fix the reader did not skip those
+ * bytes, so the stream misaligned and the following file entry was lost; with
+ * the fix the trailing bytes are skipped and helloworld.txt is read normally.
+ */
+DEFINE_TEST(test_read_format_rar5_main_block_extra_bytes)
+{
+       const char helloworld_txt[] = "hello libarchive test suite!\n";
+       la_ssize_t file_size = sizeof(helloworld_txt) - 1;
+       char buff[64];
+
+       /* HEAD_MAIN block padded with trailing bytes the parser does not read. */
+       PROLOGUE("test_read_format_rar5_main_block_extra_bytes.rar");
+
+       assertA(0 == archive_read_next_header(a, &ae));
+       assertEqualString("helloworld.txt", archive_entry_pathname(ae));
+       assertEqualInt(file_size, archive_entry_size(ae));
+       assertA(file_size == archive_read_data(a, buff, file_size));
+       assertEqualMem(buff, helloworld_txt, file_size);
+
+       assertA(ARCHIVE_EOF == archive_read_next_header(a, &ae));
+
+       EPILOGUE();
+}
+
+DEFINE_TEST(test_read_format_rar5_skip_block_extra_bytes)
+{
+       const char helloworld_txt[] = "hello libarchive test suite!\n";
+       la_ssize_t file_size = sizeof(helloworld_txt) - 1;
+       char buff[64];
+
+       /* An unknown HFL_SKIP_IF_UNKNOWN block carrying trailing bytes is inserted
+        * before the file entry; the parser must skip the entire block. */
+       PROLOGUE("test_read_format_rar5_skip_block_extra_bytes.rar");
+
+       assertA(0 == archive_read_next_header(a, &ae));
+       assertEqualString("helloworld.txt", archive_entry_pathname(ae));
+       assertEqualInt(file_size, archive_entry_size(ae));
+       assertA(file_size == archive_read_data(a, buff, file_size));
+       assertEqualMem(buff, helloworld_txt, file_size);
+
+       assertA(ARCHIVE_EOF == archive_read_next_header(a, &ae));
+
+       EPILOGUE();
+}
diff --git a/libarchive/test/test_read_format_rar5_main_block_extra_bytes.rar.uu b/libarchive/test/test_read_format_rar5_main_block_extra_bytes.rar.uu
new file mode 100644 (file)
index 0000000..81533e4
--- /dev/null
@@ -0,0 +1,6 @@
+begin 644 test_read_format_rar5_main_block_extra_bytes.rar
+M4F%R(1H' 0 $ .>F$@$%!@ % 0& @            #@P!F,L @,+G0 $G0"D
+M@P*T0Z"5@  !#FAE;&QO=V]R;&0N='AT"@,3?@ZK6U;I#AIH96QL;R!L:6)A
+;<F-H:79E('1E<W0@<W5I=&4A"AUW5E$#!00 
+`
+end
diff --git a/libarchive/test/test_read_format_rar5_skip_block_extra_bytes.rar.uu b/libarchive/test/test_read_format_rar5_skip_block_extra_bytes.rar.uu
new file mode 100644 (file)
index 0000000..1b60edd
--- /dev/null
@@ -0,0 +1,6 @@
+begin 644 test_read_format_rar5_skip_block_extra_bytes.rar
+M4F%R(1H' 0 SDK7E"@$%!@ % 0& @ ",.6KS"A $           X, 9C+ (#
+M"YT !)T I(,"M$.@E8   0YH96QL;W=O<FQD+G1X= H#$WX.JUM6Z0X::&5L
+B;&\@;&EB87)C:&EV92!T97-T('-U:71E(0H==U91 P4$    
+`
+end