From eac15e252010c1189a5c0f461364dbe2cd2a68b1 Mon Sep 17 00:00:00 2001 From: "Dustin L. Howett" Date: Thu, 9 May 2024 18:59:17 -0500 Subject: [PATCH] rar4 reader: protect copy_from_lzss_window_to_unp() (#2172) copy_from_lzss_window_to_unp unnecessarily took an `int` parameter where both of its callers were holding a `size_t`. A lzss opcode chain could be constructed that resulted in a negative copy length, which when passed into memcpy would result in a very, very large positive number. Switching copy_from_lzss_window_to_unp to take a `size_t` allows it to properly bounds-check length. In addition, this patch also ensures that `length` is not itself larger than the destination buffer. Security: CVE-2024-20696 Reference:https://github.com/libarchive/libarchive/commit/eac15e252010c1189a5c0f461364dbe2cd2a68b1 Conflict:copy_from_lzss_window_to_unp() -> copy_from_lzss_window();adapt context --- libarchive/archive_read_support_format_rar.c | 28 +++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/libarchive/archive_read_support_format_rar.c b/libarchive/archive_read_support_format_rar.c index c2666b2..512f5a6 100644 --- a/libarchive/archive_read_support_format_rar.c +++ b/libarchive/archive_read_support_format_rar.c @@ -358,7 +358,7 @@ static int make_table_recurse(struct archive_read *, struct huffman_code *, int, struct huffman_table_entry *, int, int); static int64_t expand(struct archive_read *, int64_t); static int copy_from_lzss_window(struct archive_read *, const void **, - int64_t, int); + int64_t, size_t); static const void *rar_read_ahead(struct archive_read *, size_t, ssize_t *); /* @@ -1936,7 +1936,7 @@ read_data_compressed(struct archive_read *a, const void **buff, size_t *size, bs = rar->unp_buffer_size - rar->unp_offset; else bs = (size_t)rar->bytes_uncopied; - ret = copy_from_lzss_window(a, buff, rar->offset, (int)bs); + ret = copy_from_lzss_window(a, buff, rar->offset, bs); if (ret != ARCHIVE_OK) return (ret); rar->offset += bs; @@ -2065,7 +2065,7 @@ read_data_compressed(struct archive_read *a, const void **buff, size_t *size, bs = rar->unp_buffer_size - rar->unp_offset; else bs = (size_t)rar->bytes_uncopied; - ret = copy_from_lzss_window(a, buff, rar->offset, (int)bs); + ret = copy_from_lzss_window(a, buff, rar->offset, bs); if (ret != ARCHIVE_OK) return (ret); rar->offset += bs; @@ -2923,11 +2923,16 @@ bad_data: static int copy_from_lzss_window(struct archive_read *a, const void **buffer, - int64_t startpos, int length) + int64_t startpos, size_t length) { int windowoffs, firstpart; struct rar *rar = (struct rar *)(a->format->data); + if (length > rar->unp_buffer_size) + { + goto fatal; + } + if (!rar->unp_buffer) { if ((rar->unp_buffer = malloc(rar->unp_buffer_size)) == NULL) @@ -2939,17 +2944,17 @@ copy_from_lzss_window(struct archive_read *a, const void **buffer, } windowoffs = lzss_offset_for_position(&rar->lzss, startpos); - if(windowoffs + length <= lzss_size(&rar->lzss)) { + if(windowoffs + length <= (size_t)lzss_size(&rar->lzss)) { memcpy(&rar->unp_buffer[rar->unp_offset], &rar->lzss.window[windowoffs], length); - } else if (length <= lzss_size(&rar->lzss)) { + } else if (length <= (size_t)lzss_size(&rar->lzss)) { firstpart = lzss_size(&rar->lzss) - windowoffs; if (firstpart < 0) { archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, "Bad RAR file data"); return (ARCHIVE_FATAL); } - if (firstpart < length) { + if ((size_t)firstpart < length) { memcpy(&rar->unp_buffer[rar->unp_offset], &rar->lzss.window[windowoffs], firstpart); memcpy(&rar->unp_buffer[rar->unp_offset + firstpart], @@ -2959,9 +2964,7 @@ copy_from_lzss_window(struct archive_read *a, const void **buffer, &rar->lzss.window[windowoffs], length); } } else { - archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, - "Bad RAR file data"); - return (ARCHIVE_FATAL); + goto fatal; } rar->unp_offset += length; if (rar->unp_offset >= rar->unp_buffer_size) @@ -2969,6 +2972,11 @@ copy_from_lzss_window(struct archive_read *a, const void **buffer, else *buffer = NULL; return (ARCHIVE_OK); + +fatal: + archive_set_error(&a->archive, ARCHIVE_ERRNO_FILE_FORMAT, + "Bad RAR file data"); + return (ARCHIVE_FATAL); } static const void *