]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
rar4 reader: protect copy_from_lzss_window_to_unp() (#2172)
authorDustin L. Howett <dustin@howett.net>
Thu, 9 May 2024 23:59:17 +0000 (18:59 -0500)
committerGitHub <noreply@github.com>
Thu, 9 May 2024 23:59:17 +0000 (01:59 +0200)
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

libarchive/archive_read_support_format_rar.c

index 4fc6626cacfdb2c29941f3b9b4b26f523c2b94f8..5776df4bd944c3405e3753fc51986df15c9dcbaf 100644 (file)
@@ -432,7 +432,7 @@ static int make_table_recurse(struct archive_read *, struct huffman_code *, int,
                               struct huffman_table_entry *, int, int);
 static int expand(struct archive_read *, int64_t *);
 static int copy_from_lzss_window_to_unp(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 *);
 static int parse_filter(struct archive_read *, const uint8_t *, uint16_t,
                         uint8_t);
@@ -2060,7 +2060,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_to_unp(a, buff, rar->offset, (int)bs);
+      ret = copy_from_lzss_window_to_unp(a, buff, rar->offset, bs);
       if (ret != ARCHIVE_OK)
         return (ret);
       rar->offset += bs;
@@ -2213,7 +2213,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_to_unp(a, buff, rar->offset, (int)bs);
+    ret = copy_from_lzss_window_to_unp(a, buff, rar->offset, bs);
     if (ret != ARCHIVE_OK)
       return (ret);
     rar->offset += bs;
@@ -3094,11 +3094,16 @@ copy_from_lzss_window(struct archive_read *a, void *buffer,
 
 static int
 copy_from_lzss_window_to_unp(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)
@@ -3110,17 +3115,17 @@ copy_from_lzss_window_to_unp(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],
@@ -3130,9 +3135,7 @@ copy_from_lzss_window_to_unp(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)
@@ -3140,6 +3143,11 @@ copy_from_lzss_window_to_unp(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 *