From ca7c5606a98faa45cc87ab5292c64705d8d93a85 Mon Sep 17 00:00:00 2001 From: Grzegorz Antoniak Date: Fri, 9 Nov 2018 06:01:24 +0100 Subject: 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. --- libarchive/archive_read_support_format_rar5.c | 26 +++++++++++++---- libarchive/test/test_read_format_rar5.c | 41 +++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c index aa727928..7681c948 100644 --- 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 2196123d..6c570f1c 100644 --- 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(); +} -- cgit v1.2.1