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.

2 files changed, 31 insertions, 1 deletions

diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index a3cfa72e..cc966a51 100644
--- 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 acc90510..0c302897 100644
--- 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();
+}