From d9a8da4d9c89d0048fd46ad62d210ec9ec6e8d9c Mon Sep 17 00:00:00 2001 From: Grzegorz Antoniak Date: Thu, 9 May 2019 07:00:33 +0200 Subject: RAR5 reader: fix a potential SIGSEGV on 32-bit builds The reader was causing a SIGSEGV when the file has been declaring a specific dictionary size. Dictionary sizes above 0xFFFFFFFF bytes are overflowing size_t type on 32-bit builds. In case the file has been declaring dictionary size of 0x100000000 (so, UINT_MAX+1), the window_size variable effectively contained value of 0. Later, the memory allocation function was skipping actual allocation of 0 bytes, but still tried to unpack the data. This commit limits the dictionary window size buffer to 64MB, so it always fits in a size_t variable, and disallows a zero dictionary size for files in the header processing stage. One unit test had to be modified after this change. --- libarchive/archive_read_support_format_rar5.c | 19 +++++++++++++++++-- libarchive/test/test_read_format_rar5.c | 3 ++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c index 06b340f8..5b5b8e5a 100644 --- a/libarchive/archive_read_support_format_rar5.c +++ b/libarchive/archive_read_support_format_rar5.c @@ -1570,7 +1570,7 @@ static int process_head_file(struct archive_read* a, struct rar5* rar, size_t compression_info = 0; size_t host_os = 0; size_t name_size = 0; - uint64_t unpacked_size; + uint64_t unpacked_size, window_size; uint32_t mtime = 0, crc = 0; int c_method = 0, c_version = 0; char name_utf8_buf[MAX_NAME_IN_BYTES]; @@ -1652,12 +1652,27 @@ static int process_head_file(struct archive_read* a, struct rar5* rar, c_method = (int) (compression_info >> 7) & 0x7; c_version = (int) (compression_info & 0x3f); - rar->cstate.window_size = (rar->file.dir > 0) ? + /* RAR5 seems to limit the dictionary size to 64MB. */ + window_size = (rar->file.dir > 0) ? 0 : g_unpack_window_size << ((compression_info >> 10) & 15); rar->cstate.method = c_method; rar->cstate.version = c_version + 50; + /* Check if window_size is a sane value. Also, if the file is not + * declared as a directory, disallow window_size == 0. */ + if(window_size > (64 * 1024 * 1024) || + (rar->file.dir == 0 && window_size == 0)) + { + archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, + "Declared dictionary size is not supported."); + return ARCHIVE_FATAL; + } + + /* Values up to 64M should fit into ssize_t on every + * architecture. */ + rar->cstate.window_size = (ssize_t) window_size; + rar->file.solid = (compression_info & SOLID) > 0; rar->file.service = 0; diff --git a/libarchive/test/test_read_format_rar5.c b/libarchive/test/test_read_format_rar5.c index 29af4ea6..40ba3c5d 100644 --- a/libarchive/test/test_read_format_rar5.c +++ b/libarchive/test/test_read_format_rar5.c @@ -1039,7 +1039,8 @@ DEFINE_TEST(test_read_format_rar5_invalid_dict_reference) PROLOGUE("test_read_format_rar5_invalid_dict_reference.rar"); - assertA(0 == archive_read_next_header(a, &ae)); + /* This test should fail on parsing the header. */ + assertA(archive_read_next_header(a, &ae) != ARCHIVE_OK); /* This archive is invalid. However, processing it shouldn't cause any * errors related to buffer underflow when using -fsanitize. */ -- cgit v1.2.1