]> git.ipfire.org Git - thirdparty/libarchive.git/commitdiff
RAR5 reader: fix bad shift-left operations.
authorGrzegorz Antoniak <ga@anadoxin.org>
Wed, 1 May 2019 04:47:31 +0000 (06:47 +0200)
committerGrzegorz Antoniak <ga@anadoxin.org>
Wed, 1 May 2019 04:47:31 +0000 (06:47 +0200)
This commit fixes some undefined shift-left operations on types that do
not support such a big shift. Those invalid shift operations were
triggering on invalid files produced by fuzzing.

The commit also contains two unit tests that ensure such problems won't
arise in the future.

Fixes OSSFuzz cases #14490 and #14491.

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

index cf897657caf36d81657b1c8482f8e4d438a90f87..aac74cd47021085f5d93e0ff60667468ba5ad91a 100644 (file)
@@ -2524,7 +2524,9 @@ static int parse_filter_data(struct rar5* rar, const uint8_t* p,
             return ARCHIVE_EOF;
         }
 
-        data += (byte >> 8) << (i * 8);
+        /* Cast to uint32_t will ensure the shift operation will not produce
+         * undefined result. */
+        data += ((uint32_t) byte >> 8) << (i * 8);
         skip_bits(rar, 8);
     }
 
@@ -2748,7 +2750,11 @@ static int do_uncompress_block(struct archive_read* a, const uint8_t* p) {
                 dist += dist_slot;
             } else {
                 dbits = dist_slot / 2 - 1;
-                dist += (2 | (dist_slot & 1)) << dbits;
+
+                /* Cast to uint32_t will make sure the shift left operation
+                 * won't produce undefined result. Then, the uint32_t type will
+                 * be implicitly casted to int. */
+                dist += (uint32_t) (2 | (dist_slot & 1)) << dbits;
             }
 
             if(dbits > 0) {
index d52b6002c54d45df905bfa75dcbb39304839c700..9b03af13bff36ec367c4d55eb2752733070263f2 100644 (file)
@@ -965,3 +965,33 @@ DEFINE_TEST(test_read_format_rar5_readtables_overflow)
 
     EPILOGUE();
 }
+
+DEFINE_TEST(test_read_format_rar5_leftshift1)
+{
+    uint8_t buf[16];
+
+    PROLOGUE("test_read_format_rar5_leftshift1.rar");
+
+    assertA(0 == archive_read_next_header(a, &ae));
+    /* This archive is invalid. However, processing it shouldn't cause any
+     * errors related to undefined operations when using -fsanitize. */
+    assertA(ARCHIVE_FATAL == archive_read_data(a, buf, sizeof(buf)));
+    assertA(ARCHIVE_EOF == archive_read_next_header(a, &ae));
+
+    EPILOGUE();
+}
+
+DEFINE_TEST(test_read_format_rar5_leftshift2)
+{
+    uint8_t buf[16];
+
+    PROLOGUE("test_read_format_rar5_leftshift2.rar");
+
+    assertA(0 == archive_read_next_header(a, &ae));
+    /* This archive is invalid. However, processing it shouldn't cause any
+     * errors related to undefined operations when using -fsanitize. */
+    assertA(ARCHIVE_FATAL == archive_read_data(a, buf, sizeof(buf)));
+    assertA(ARCHIVE_EOF == archive_read_next_header(a, &ae));
+
+    EPILOGUE();
+}
diff --git a/libarchive/test/test_read_format_rar5_leftshift1.rar.uu b/libarchive/test/test_read_format_rar5_leftshift1.rar.uu
new file mode 100644 (file)
index 0000000..694a27f
--- /dev/null
@@ -0,0 +1,9 @@
+begin 644 test_read_format_rar5_leftshift1.rar
+M4F%R(1H'`0"-[P+2``(''(`'`"``_R4``B$<`0(`#@```0```"#2````____
+M_P`(`/__^P#_W0`"(8#_`(:&;;%DS+?,L=:NL0#(3`$`````````````````
+M``"``````````+!DS+*RL[*RL@```-P``````````````````(``````````
+ML&3,LK*RLK*R````W`````#X____````````````````````````%5H>;&@T
+M+3HW"2!SB^)_<Z3_`````?40'Q\?'Q\?'Q\?'Q\?'Q\?'Q\?'Q\?'Q\`````
+5`````````````````/H`>@``````
+`
+end
diff --git a/libarchive/test/test_read_format_rar5_leftshift2.rar.uu b/libarchive/test/test_read_format_rar5_leftshift2.rar.uu
new file mode 100644 (file)
index 0000000..57ffad7
--- /dev/null
@@ -0,0 +1,6 @@
+begin 644 test_read_format_rar5_leftshift2.rar
+M4F%R(1H'`0"-[P+2``(''(`'`2``_RL``B'+`0(`,O__````-WJ\KR<<)0`"
+M(;<*`BY*`!```&;%T#%24%"`_R4`[@K+(2Y*`&$``'__`/\E``(N2@`0`0(`
+0(?__`%N&?Q2UH.CHZ.CHZ```
+`
+end