]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
RAR5 reader: fix a potential SIGSEGV on 32-bit builds 1196/head
authorGrzegorz Antoniak <ga@anadoxin.org>
Thu, 9 May 2019 05:00:33 +0000 (07:00 +0200)
committerGrzegorz Antoniak <ga@anadoxin.org>
Thu, 9 May 2019 05:00:33 +0000 (07:00 +0200)
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
libarchive/test/test_read_format_rar5.c

index 06b340f8d16e62021e8374eac95cf647692b9fa8..5b5b8e5a3e6fd599f8a0bb33d4376794892eb9df 100644 (file)
@@ -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;
 
index 29af4ea61f7ad963f78770f7828662245d521c96..40ba3c5d0f7b598c2e2688f0a9802964f7dc7a9b 100644 (file)
@@ -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. */