rar5: Fix random initial offset if using archive_read_data_into_fd

archive_read_data_into_fd passes a pointer to an uninitialized
variable as an output 'offset' argument into archive_read_data_block
function, and expects that this variable will always be initialized
inside of it.

Like this:
  size_t size;
  int64_t offset;
  archive_read_data_block(a, &buf, &size, &offset);
  /* some work with offset here */

But rar5 implementation of archive_read_data_block function leaves the
'offset' argument uninitialized in one code path (if file is
compressed and there are no uncompressed pending data blocks).

As a result, archive_read_data_info_fd function is using an
uninitialized variable as an initial offset of an output file. And in
most cases it causes an appending sparse block of a random size at the
beginning of the output file.

diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index a3cfa72e77c8c31bdcedecbaabb36fc1f3257ec3..cc966a51ee0deb28d9760f6ba79f1b71886a35b8 100644 (file)
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -3911,6 +3911,13 @@ static int do_unpack(struct archive_read* a, struct rar5* rar,
                        case GOOD:
                                /* fallthrough */
                        case BEST:
+                               /* No data is returned here. But because a sparse-file aware
+                                * caller (like archive_read_data_into_fd) may treat zero-size
+                                * as a sparse file block, we need to update the offset
+                                * accordingly. At this point the decoder doesn't have any
+                                * pending uncompressed data blocks, so the current position in
+                                * the output file should be last_write_ptr. */
+                               if (offset) *offset = rar->cstate.last_write_ptr;
                                return uncompress_file(a);
                        default:
                                archive_set_error(&a->archive,

diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c
index acc90510946bd25e2dc233413825ae36817a1149..0c3028971bbc9d86433b4ef60d1fbbcf281b1fca 100644 (file)
--- a/libarchive/test/test_read_format_rar5.c
+++ b/libarchive/test/test_read_format_rar5.c
@@ -1346,4 +1346,27 @@ DEFINE_TEST(test_read_format_rar5_bad_window_size_in_multiarchive_file)
        while(0 < archive_read_data(a, buf, sizeof(buf))) {}
 
        EPILOGUE();
-}
\ No newline at end of file
+}
+
+DEFINE_TEST(test_read_format_rar5_read_data_block_uninitialized_offset)
+{
+       const void *buf;
+       size_t size;
+       la_int64_t offset;
+
+       PROLOGUE("test_read_format_rar5_compressed.rar");
+       assertA(0 == archive_read_next_header(a, &ae));
+
+       /* A real code may pass a pointer to an uninitialized variable as an offset
+        * output argument. Here we want to check this situation. But because
+        * relying on a value of an uninitialized variable in a test is not a good
+        * idea, let's pretend that 0xdeadbeef is a random value of the
+        * uninitialized variable. */
+       offset = 0xdeadbeef;
+       assertEqualInt(ARCHIVE_OK, archive_read_data_block(a, &buf, &size, &offset));
+       /* The test archive doesn't contain a sparse file. And because of that, here
+        * we assume that the first returned offset should be 0. */
+       assertEqualInt(0, offset);
+
+       EPILOGUE();
+}