RAR5 reader: Fixed a read from invalid memory block

In multi-file RAR5 archives, if a block spans from one file to another,
the RAR5 reader merges both blocks into one, and feeds this merged block
to the decompressor function. The problem is that the block merge
function allocates the exact number of bytes for this block. This is
problematic because when trying to read the last byte from this new
block with bit reader functions, the bit reader functions will reference
few additional bytes right after the byte the caller is trying to read,
resulting in an out of bounds read.

The commit increases the allocation size for new merged block. This
ensures that bit reader functions will never perform any out of bounds
reads. Additional space is zeroed out to prevent errors from
instrumentation tools like ASan or Valgrind.

Fixes #1119

diff --git a/libarchive/archive_read_support_format_rar5.c b/libarchive/archive_read_support_format_rar5.c
index 99d8176818cca15fdd7d50e97a725e0f68bc8007..6d824673e399e0012098954a994ff5cb414a53c2 100644 (file)
--- a/libarchive/archive_read_support_format_rar5.c
+++ b/libarchive/archive_read_support_format_rar5.c
@@ -2679,13 +2679,21 @@ static int merge_block(struct archive_read* a, ssize_t block_size,
     if(rar->vol.push_buf)
         free((void*) rar->vol.push_buf);
 
-    rar->vol.push_buf = malloc(block_size);
+    /* Increasing the allocation block by 8 is due to bit reading functions,
+     * which are using additional 2 or 4 bytes. Allocating the block size
+     * by exact value would make bit reader perform reads from invalid memory
+     * block when reading the last byte from the buffer. */
+    rar->vol.push_buf = malloc(block_size + 8);
     if(!rar->vol.push_buf) {
         archive_set_error(&a->archive, ENOMEM, "Can't allocate memory for a "
                 "merge block buffer.");
         return ARCHIVE_FATAL;
     }
 
+    /* Valgrind complains if the extension block for bit reader is not
+     * initialized, so initialize it. */
+    memset(&rar->vol.push_buf[block_size], 0, 8);
+
     /* A single block can span across multiple multivolume archive files,
      * so we use a loop here. This loop will consume enough multivolume
      * archive files until the whole block is read. */