]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
RAR5 reader: fix invalid type used for dictionary size mask.
authorGrzegorz Antoniak <ga@anadoxin.org>
Fri, 3 May 2019 06:31:28 +0000 (08:31 +0200)
committerGrzegorz Antoniak <ga@anadoxin.org>
Fri, 3 May 2019 06:31:28 +0000 (08:31 +0200)
This commit fixes places where the window_mask variable, which is needed
to perform operations on the dictionary circular buffer, was casted to
an int variable.

In files that declare dictionary buffer size of 4GB, window_mask has a
value of 0xFFFFFFFF. If this value will be assigned to an int variable,
this will effectively make the variable to contain value of -1. This
means, that any cast to a 64-bit value will bit-extend the int variable
to 0xFFFFFFFFFFFFFFFF. This was happening during a read operation from
the dictionary. Such invalid window_mask variable was not guarding
against buffer underflow.

This commit should fix the OSSFuzz issue #14537.

The commit also contains a test case for this issue.

libarchive/archive_read_support_format_rar5.c
libarchive/test/test_read_format_rar5.c
libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu [new file with mode: 0644]

index e3c3b62d19e820299f0489fa37203a5ed6983e2a..9f6dc6437cff02321610a60e83158cda4c313735 100644 (file)
@@ -505,7 +505,7 @@ static void write_filter_data(struct rar5* rar, uint32_t offset,
     archive_le32enc(&rar->cstate.filtered_buf[offset], value);
 }
 
-static void circular_memcpy(uint8_t* dst, uint8_t* window, const int mask,
+static void circular_memcpy(uint8_t* dst, uint8_t* window, const uint64_t mask,
         int64_t start, int64_t end)
 {
     if((start & mask) > (end & mask)) {
@@ -562,16 +562,15 @@ static int run_e8e9_filter(struct rar5* rar, struct filter_info* flt,
     const uint32_t file_size = 0x1000000;
     ssize_t i;
 
-    const int mask = (int)rar->cstate.window_mask;
     circular_memcpy(rar->cstate.filtered_buf,
         rar->cstate.window_buf,
-        mask,
+        rar->cstate.window_mask,
         rar->cstate.solid_offset + flt->block_start,
         rar->cstate.solid_offset + flt->block_start + flt->block_length);
 
     for(i = 0; i < flt->block_length - 4;) {
         uint8_t b = rar->cstate.window_buf[(rar->cstate.solid_offset +
-                flt->block_start + i++) & mask];
+                flt->block_start + i++) & rar->cstate.window_mask];
 
         /* 0xE8 = x86's call <relative_addr_uint32> (function call)
          * 0xE9 = x86's jmp <relative_addr_uint32> (unconditional jump) */
@@ -604,22 +603,21 @@ static int run_e8e9_filter(struct rar5* rar, struct filter_info* flt,
 static int run_arm_filter(struct rar5* rar, struct filter_info* flt) {
     ssize_t i = 0;
     uint32_t offset;
-    const int mask = (int)rar->cstate.window_mask;
 
     circular_memcpy(rar->cstate.filtered_buf,
         rar->cstate.window_buf,
-        mask,
+        rar->cstate.window_mask,
         rar->cstate.solid_offset + flt->block_start,
         rar->cstate.solid_offset + flt->block_start + flt->block_length);
 
     for(i = 0; i < flt->block_length - 3; i += 4) {
         uint8_t* b = &rar->cstate.window_buf[(rar->cstate.solid_offset +
-                flt->block_start + i) & mask];
+                flt->block_start + i) & rar->cstate.window_mask];
 
         if(b[3] == 0xEB) {
             /* 0xEB = ARM's BL (branch + link) instruction. */
             offset = read_filter_data(rar, (rar->cstate.solid_offset +
-                        flt->block_start + i) & mask) & 0x00ffffff;
+                        flt->block_start + i) & rar->cstate.window_mask) & 0x00ffffff;
 
             offset -= (uint32_t) ((i + flt->block_start) / 4);
             offset = (offset & 0x00ffffff) | 0xeb000000;
@@ -689,7 +687,7 @@ static int run_filter(struct archive_read* a, struct filter_info* flt) {
 static void push_data(struct archive_read* a, struct rar5* rar,
         const uint8_t* buf, int64_t idx_begin, int64_t idx_end)
 {
-    const int wmask = (int)rar->cstate.window_mask;
+    const uint64_t wmask = rar->cstate.window_mask;
     const ssize_t solid_write_ptr = (rar->cstate.solid_offset +
         rar->cstate.last_write_ptr) & wmask;
 
@@ -2660,8 +2658,8 @@ static int decode_code_length(struct rar5* rar, const uint8_t* p,
 
 static int copy_string(struct archive_read* a, int len, int dist) {
     struct rar5* rar = get_context(a);
-    const int cmask = (int)rar->cstate.window_mask;
-    const int64_t write_ptr = rar->cstate.write_ptr + rar->cstate.solid_offset;
+    const uint64_t cmask = rar->cstate.window_mask;
+    const uint64_t write_ptr = rar->cstate.write_ptr + rar->cstate.solid_offset;
     int i;
 
     /* The unpacker spends most of the time in this function. It would be
@@ -2686,7 +2684,7 @@ static int do_uncompress_block(struct archive_read* a, const uint8_t* p) {
     uint16_t num;
     int ret;
 
-    const int cmask = (int)rar->cstate.window_mask;
+    const uint64_t cmask = rar->cstate.window_mask;
     const struct compressed_block_header* hdr = &rar->last_block_hdr;
     const uint8_t bit_size = 1 + bf_bit_size(hdr);
 
index cc1c4b896e075758df9fd2b5416d43cb3730351d..e53cf061a47f30b9a561d6125b856fecf352446a 100644 (file)
@@ -1010,3 +1010,18 @@ DEFINE_TEST(test_read_format_rar5_truncated_huff)
 
     EPILOGUE();
 }
+
+DEFINE_TEST(test_read_format_rar5_invalid_dict_reference)
+{
+    uint8_t buf[16];
+
+    PROLOGUE("test_read_format_rar5_invalid_dict_reference.rar");
+
+    assertA(0 == archive_read_next_header(a, &ae));
+    /* This archive is invalid. However, processing it shouldn't cause any
+     * errors related to buffer underflow when using -fsanitize. */
+    assertA(ARCHIVE_FATAL == archive_read_data(a, buf, sizeof(buf)));
+    assertA(ARCHIVE_EOF == archive_read_next_header(a, &ae));
+
+    EPILOGUE();
+}
\ No newline at end of file
diff --git a/libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu b/libarchive/test/test_read_format_rar5_invalid_dict_reference.rar.uu
new file mode 100644 (file)
index 0000000..9b78c9b
--- /dev/null
@@ -0,0 +1,9 @@
+begin 644 test_read_format_rar5_invalid_dict_reference.rar
+M4F%R(1H'`0"-[P+2``+#!QR`!P`F^P#_^_O[^_O[^R4``B$<`0(`#@```0``
+M`"#2````_____QH(`/__^P#_W5)04(#_`(:&;;%DS+?,L0```````````+%D
+MS+*RLK*R/@``____Y`"R````XP```````!4``````.X`````````````````
+M%5<M;&@W;3$W"2!S;'$2C5L`_____@D0````$"('``"8F)@+````/__?````
+M@```2$A(2$A(2$A(2$A(2$A(2$A(2$A(2$A(2$A(2$A(2$@S2(``2$A(2$A(
+>2$A(2$A(2$A(2$A(2$A(2$Q(2$A(2$A(2$A(2)](
+`
+end