Skip to content

Commit 267cda9

Browse files
committed
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.
1 parent 1385cd9 commit 267cda9

2 files changed

Lines changed: 31 additions & 1 deletion

File tree

libarchive/archive_read_support_format_rar5.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3911,6 +3911,13 @@ static int do_unpack(struct archive_read* a, struct rar5* rar,
39113911
case GOOD:
39123912
/* fallthrough */
39133913
case BEST:
3914+
/* No data is returned here. But because a sparse-file aware
3915+
* caller (like archive_read_data_into_fd) may treat zero-size
3916+
* as a sparse file block, we need to update the offset
3917+
* accordingly. At this point the decoder doesn't have any
3918+
* pending uncompressed data blocks, so the current position in
3919+
* the output file should be last_write_ptr. */
3920+
if (offset) *offset = rar->cstate.last_write_ptr;
39143921
return uncompress_file(a);
39153922
default:
39163923
archive_set_error(&a->archive,

libarchive/test/test_read_format_rar5.c

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1346,4 +1346,27 @@ DEFINE_TEST(test_read_format_rar5_bad_window_size_in_multiarchive_file)
13461346
while(0 < archive_read_data(a, buf, sizeof(buf))) {}
13471347

13481348
EPILOGUE();
1349-
}
1349+
}
1350+
1351+
DEFINE_TEST(test_read_format_rar5_read_data_block_uninitialized_offset)
1352+
{
1353+
const void *buf;
1354+
size_t size;
1355+
la_int64_t offset;
1356+
1357+
PROLOGUE("test_read_format_rar5_compressed.rar");
1358+
assertA(0 == archive_read_next_header(a, &ae));
1359+
1360+
/* A real code may pass a pointer to an uninitialized variable as an offset
1361+
* output argument. Here we want to check this situation. But because
1362+
* relying on a value of an uninitialized variable in a test is not a good
1363+
* idea, let's pretend that 0xdeadbeef is a random value of the
1364+
* uninitialized variable. */
1365+
offset = 0xdeadbeef;
1366+
assertEqualInt(ARCHIVE_OK, archive_read_data_block(a, &buf, &size, &offset));
1367+
/* The test archive doesn't contain a sparse file. And because of that, here
1368+
* we assume that the first returned offset should be 0. */
1369+
assertEqualInt(0, offset);
1370+
1371+
EPILOGUE();
1372+
}

0 commit comments

Comments
 (0)