RAR5 reader bugfixes (block-by-block, loops, warnings)

- Fixed a bug during a block-by-block reading loop. Added a test that
checks for the existence of this bug.

- Fixed 2 unlimited loops encountered when unpacking corrupted data.

- Removed some 'maybe uninitialized' warnings.

diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index aa72792808af96ff4d7b42db7b270225a23d50bc..7681c94861476125dcd393177758145e66aa510b 100644 (file)
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -88,6 +88,7 @@ struct file_header {
 
     uint8_t solid : 1;           /* Is this a solid stream? */
     uint8_t service : 1;         /* Is this file a service data? */
+    uint8_t eof : 1;             /* Did we finish unpacking the file? */
 
     /* Optional time fields. */
     uint64_t e_mtime;
@@ -918,7 +919,7 @@ static int read_var_sized(struct archive_read* a, size_t* pvalue,
         size_t* pvalue_len)
 {
     uint64_t v;
-    uint64_t v_size;
+    uint64_t v_size = 0;
 
     const int ret = pvalue_len
                     ? read_var(a, &v, &v_size)
@@ -1218,7 +1219,7 @@ static int process_head_file_extra(struct archive_read* a,
         ssize_t extra_data_size)
 {
     size_t extra_field_size;
-    size_t extra_field_id;
+    size_t extra_field_id = 0;
     int ret = ARCHIVE_FATAL;
     size_t var_size;
 
@@ -1288,7 +1289,7 @@ static int process_head_file(struct archive_read* a, struct rar5* rar,
     size_t host_os = 0;
     size_t name_size = 0;
     uint64_t unpacked_size;
-    uint32_t mtime = 0, crc;
+    uint32_t mtime = 0, crc = 0;
     int c_method = 0, c_version = 0, is_dir;
     char name_utf8_buf[2048 * 4];
     const uint8_t* p;
@@ -1647,7 +1648,7 @@ static int process_base_block(struct archive_read* a,
 {
     struct rar5* rar = get_context(a);
     uint32_t hdr_crc, computed_crc;
-    size_t raw_hdr_size, hdr_size_len, hdr_size;
+    size_t raw_hdr_size = 0, hdr_size_len, hdr_size;
     size_t header_id = 0;
     size_t header_flags = 0;
     const uint8_t* p;
@@ -2685,6 +2686,12 @@ static int merge_block(struct archive_read* a, ssize_t block_size,
         cur_block_size =
             rar5_min(rar->file.bytes_remaining, block_size - partial_offset);
 
+        if(cur_block_size == 0) {
+            archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT,
+                    "Encountered block size == 0 during block merge");
+            return ARCHIVE_FATAL;
+        }
+
         if(!read_ahead(a, cur_block_size, &lp))
             return ARCHIVE_EOF;
 
@@ -3116,6 +3123,9 @@ static int do_unstore_file(struct archive_read* a,
     }
 
     size_t to_read = rar5_min(rar->file.bytes_remaining, 64 * 1024);
+    if(to_read == 0) {
+        return ARCHIVE_EOF;
+    }
 
     if(!read_ahead(a, to_read, &p)) {
         archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, "I/O error "
@@ -3283,8 +3293,13 @@ static int rar5_read_data(struct archive_read *a, const void **buff,
     }
 
     ret = use_data(rar, buff, size, offset);
-    if(ret == ARCHIVE_OK)
+    if(ret == ARCHIVE_OK) {
         return ret;
+    }
+
+    if(rar->file.eof == 1) {
+        return ARCHIVE_EOF;
+    }
 
     ret = do_unpack(a, rar, buff, size, offset);
     if(ret != ARCHIVE_OK) {
@@ -3301,6 +3316,7 @@ static int rar5_read_data(struct archive_read *a, const void **buff,
          * value in the last `archive_read_data` call to signal an error
          * to the user. */
 
+        rar->file.eof = 1;
         return verify_global_checksums(a);
     }
 

diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c
index 2196123ddbc5882b865c5dcc9a44b1fc777b106c..6c570f1c66436a3da78645a60dccc5c8b0201247 100644 (file)
--- a/libarchive/test/test_read_format_rar5.c
+++ b/libarchive/test/test_read_format_rar5.c
@@ -726,3 +726,44 @@ DEFINE_TEST(test_read_format_rar5_extract_win32)
     assertA(0 == extract_one(a, ae, 0x36A448FF));
     EPILOGUE();
 }
+
+DEFINE_TEST(test_read_format_rar5_block_by_block)
+{
+    /* This test uses strange buffer sizes intentionally. */
+
+    struct archive_entry *ae;
+    struct archive *a;
+    uint8_t buf[173];
+    int bytes_read;
+    uint32_t computed_crc = 0;
+
+    extract_reference_file("test_read_format_rar5_compressed.rar");
+    assert((a = archive_read_new()) != NULL);
+    assertA(0 == archive_read_support_filter_all(a));
+    assertA(0 == archive_read_support_format_all(a));
+    assertA(0 == archive_read_open_filename(a, "test_read_format_rar5_compressed.rar", 130));
+    assertA(0 == archive_read_next_header(a, &ae));
+    assertEqualString("test.bin", archive_entry_pathname(ae));
+    assertEqualInt(1200, archive_entry_size(ae));
+
+    /* File size is 1200 bytes, we're reading it using a buffer of 173 bytes.
+     * Libarchive is configured to use a buffer of 130 bytes. */
+
+    while(1) {
+        /* archive_read_data should return one of:
+         * a) 0, if there is no more data to be read,
+         * b) negative value, if there was an error,
+         * c) positive value, meaning how many bytes were read.
+         */
+
+        bytes_read = archive_read_data(a, buf, sizeof(buf));
+        assertA(bytes_read >= 0);
+        if(bytes_read <= 0)
+            break;
+
+        computed_crc = crc32(computed_crc, buf, bytes_read);
+    }
+
+    assertEqualInt(computed_crc, 0x7CCA70CD);
+    EPILOGUE();
+}