[zlib] Adjust the scan[2]==match[2] assert for CRC hashing

When using CRC hashing, that assert doesn't hold, but a weaker variant
does.

Also disallow compiling with FASTEST defined, because I don't think
that longest_match variant would be safe with CRC hashing. It doesn't
guarantee that the fourth of the hashed bytes will be compared.

Bug: 1113596
Change-Id: I20ede29835b9c6b4bdc09fc1836864ffa2b10a97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2401023
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: Adenilson Cavalcanti <cavalcantii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805690}

third_party/zlib/contrib/tests/utils_unittest.cc

@@ -210,3 +210,72 @@ TEST(ZlibTest, CRCHashBitsCollision) {
 
   EXPECT_EQ(src, decompressed);
 }
+
+TEST(ZlibTest, CRCHashAssert) {
+  // The CRC32c of the hex sequences ff,ff,5e,6f and ff,ff,13,ff have the same
+  // lower 15 bits. This means longest_match's assert that match[2] == scan[2]
+  // won't hold. However, such hash collisions are only possible when one of the
+  // other four bytes also mismatch. This tests that zlib's assert handles this
+  // case.
+
+  std::vector<uint8_t> src = {
+      // Random byte; zlib doesn't match at offset 0.
+      123,
+
+      // This has the same hash as the last byte sequence, and the first two and
+      // last two bytes match; though the third and the fourth don't.
+      0xff,
+      0xff,
+      0x5e,
+      0x6f,
+      0x12,
+      0x34,
+
+      // Offer a 5-byte match to bump the next expected match length to 6
+      // (because the two first and two last bytes need to match).
+      0xff,
+      0xff,
+      0x13,
+      0xff,
+      0x12,
+
+      0xff,
+      0xff,
+      0x13,
+      0xff,
+      0x12,
+      0x34,
+  };
+
+  z_stream stream;
+  stream.zalloc = nullptr;
+  stream.zfree = nullptr;
+
+  int ret = deflateInit2(&stream, /*comp level*/ 5, /*method*/ Z_DEFLATED,
+                         /*windowbits*/ -15, /*memlevel*/ 8,
+                         /*strategy*/ Z_DEFAULT_STRATEGY);
+  ASSERT_EQ(ret, Z_OK);
+  std::vector<uint8_t> compressed(100, '\0');
+  stream.next_out = compressed.data();
+  stream.avail_out = compressed.size();
+  stream.next_in = src.data();
+  stream.avail_in = src.size();
+  ret = deflate(&stream, Z_FINISH);
+  ASSERT_EQ(ret, Z_STREAM_END);
+  compressed.resize(compressed.size() - stream.avail_out);
+  deflateEnd(&stream);
+
+  ret = inflateInit2(&stream, /*windowbits*/ -15);
+  ASSERT_EQ(ret, Z_OK);
+  std::vector<uint8_t> decompressed(src.size(), '\0');
+  stream.next_in = compressed.data();
+  stream.avail_in = compressed.size();
+  stream.next_out = decompressed.data();
+  stream.avail_out = decompressed.size();
+  ret = inflate(&stream, Z_FINISH);
+  ASSERT_EQ(ret, Z_STREAM_END);
+  EXPECT_EQ(0U, stream.avail_out);
+  inflateEnd(&stream);
+
+  EXPECT_EQ(src, decompressed);
+}

third_party/zlib/deflate.c

@@ -60,6 +60,11 @@
 #include "crc32_simd.h"
 #endif
 
+#ifdef FASTEST
+/* See http://crbug.com/1113596 */
+#error "FASTEST is not supported in Chromium's zlib."
+#endif
+
 const char deflate_copyright[] =
    " deflate 1.2.11 Copyright 1995-2017 Jean-loup Gailly and Mark Adler ";
 /*
@@ -1345,7 +1350,16 @@ local uInt longest_match(s, cur_match)
          * necessary to put more guard bytes at the end of the window, or
          * to check more often for insufficient lookahead.
          */
+        if (!x86_cpu_enable_simd && !arm_cpu_enable_crc32) {
           Assert(scan[2] == match[2], "scan[2]?");
+        } else {
+          /* When using CRC hashing, scan[2] and match[2] may mismatch, but in
+           * that case at least one of the other hashed bytes will mismatch
+           * also. Bytes 0 and 1 were already checked above, and we know there
+           * are at least four bytes to check otherwise the mismatch would have
+           * been found by the scan_end comparison above, so: */
+          Assert(scan[2] == match[2] || scan[3] != match[3], "scan[2]??");
+        }
         scan++, match++;
         do {
         } while (*(ushf*)(scan+=2) == *(ushf*)(match+=2) &&
@@ -1376,7 +1390,16 @@ local uInt longest_match(s, cur_match)
          * the hash keys are equal and that HASH_BITS >= 8.
          */
         scan += 2, match++;
+        if (!x86_cpu_enable_simd && !arm_cpu_enable_crc32) {
           Assert(*scan == *match, "match[2]?");
+        } else {
+          /* When using CRC hashing, scan[2] and match[2] may mismatch, but in
+           * that case at least one of the other hashed bytes will mismatch
+           * also. Bytes 0 and 1 were already checked above, and we know there
+           * are at least four bytes to check otherwise the mismatch would have
+           * been found by the scan_end comparison above, so: */
+          Assert(*scan == *match || scan[1] != match[1], "match[2]??");
+        }
 
         /* We check for insufficient lookahead only every 8th comparison;
          * the 256th check will be made at strstart+258.