fix #197, oss-fuzz/10036: only write 4 bytes per iteration in deflate_quick

by aggregating the two consecutive values to be written by static_emit_ptr to
s->pending_buf and writing the two values at once in a 4 byte store, we avoid
running out of the allocated buffer. We used to call quick_send_bits twice and
bumped the counter s->pending in the first call, which made the second call
write to memory beyond the safe 4 bytes that were guaranteed by the following
condition in the enclosing loop in deflate_quick:

  if (s->pending + 4 >= s->pending_buf_size) {
    flush_pending(s->strm);

The bug was exposed by the memory sanitizer like so:

MemorySanitizer:DEADLYSIGNAL
--
  | ==1==ERROR: MemorySanitizer: SEGV on unknown address 0x730000020000 (pc 0x0000005b6ce4 bp 0x7fff59adb5e0 sp 0x7fff59adb570 T1)
  | ==1==The signal is caused by a WRITE memory access.
  | #0 0x5b6ce3 in quick_send_bits zlib-ng/arch/x86/deflate_quick.c:134:48
  | #1 0x5b5752 in deflate_quick zlib-ng/arch/x86/deflate_quick.c:243:21
  | #2 0x590a15 in zng_deflate zlib-ng/deflate.c:952:18
  | #3 0x587165 in zng_compress2 zlib-ng/compress.c:59:15
  | #4 0x5866d3 in check_compress_level zlib-ng/test/fuzz/compress_fuzzer.c:22:3
  | #5 0x5862d8 in LLVMFuzzerTestOneInput zlib-ng/test/fuzz/compress_fuzzer.c:74:3
  | #6 0x4e9b48 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:575:15
  | #7 0x4a2f66 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:280:6
  | #8 0x4b3adb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:715:9
  | #9 0x4a2091 in main /src/libfuzzer/FuzzerMain.cpp:20:10
  | #10 0x7fb8919b082f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291
  | #11 0x41ec68 in _start
  | MemorySanitizer can not provide additional info.
  | SUMMARY: MemorySanitizer: SEGV (/mnt/scratch0/clusterfuzz/slave-bot/builds/clusterfuzz-builds_zlib-ng_7ead0a3e4980f024583384fd355b6e3ddd4b2ca2/revisions/compress_fuzzer+0x5b6ce3)

diff --git a/arch/x86/deflate_quick.c b/arch/x86/deflate_quick.c
index dc72ca5ad62257958364a27913d86b2719d2d6f0..bb9ca280f5e3c76610c592503618561ecd07a4f6 100644 (file)
--- a/arch/x86/deflate_quick.c
+++ b/arch/x86/deflate_quick.c
@@ -123,41 +123,51 @@ static inline long compare258(const unsigned char *const src0, const unsigned ch
 static const unsigned quick_len_codes[MAX_MATCH-MIN_MATCH+1];
 static const unsigned quick_dist_codes[8192];
 
-static inline void quick_send_bits(deflate_state *const s, const int value, const int length) {
-    unsigned out, width, bytes_out;
+static inline void quick_send_bits(deflate_state *const s,
+                                   const int value1, const int length1,
+                                   const int value2, const int length2) {
+    unsigned offset2 = s->bi_valid + length1;
+    unsigned width = s->bi_valid + length1 + length2;
+    unsigned bytes_out = width / 8;
 
     /* Concatenate the new bits with the bits currently in the buffer */
-    out = s->bi_buf | (value << s->bi_valid);
-    width = s->bi_valid + length;
+    unsigned out = s->bi_buf | (value1 << s->bi_valid);
+    if (width < 32) {
+      out |= (value2 << offset2);
+      /* Shift out the valid LSBs written out. */
+      s->bi_buf = out >> (bytes_out * 8);
+    } else /* width => 32 */ {
+      unsigned bits_that_fit = 32 - offset2;
+      unsigned mask = (1 << bits_that_fit) - 1;
+      /* Zero out the high bits of value2 such that the shift by offset2 will
+         not cause undefined behavior. */
+      out |= ((value2 & mask) << offset2);
+
+      /* Save in s->bi_buf the bits of value2 that do not fit: they will be
+         written in a next full byte. */
+      s->bi_buf = (width == 32) ? 0 : value2 >> bits_that_fit;
+    }
+
+    s->bi_valid = width - (bytes_out * 8);
 
     /* Taking advantage of the fact that LSB comes first, write to output buffer */
     *(unsigned *)(s->pending_buf + s->pending) = out;
 
-    bytes_out = width / 8;
-
     s->pending += bytes_out;
-
-    /* Shift out the valid LSBs written out */
-    s->bi_buf =  out >> (bytes_out * 8);
-    s->bi_valid = width - (bytes_out * 8);
 }
 
 static inline void static_emit_ptr(deflate_state *const s, const int lc, const unsigned dist) {
-    unsigned code, len;
-
-    code = quick_len_codes[lc] >> 8;
-    len =  quick_len_codes[lc] & 0xFF;
-    quick_send_bits(s, code, len);
-
-    code = quick_dist_codes[dist-1] >> 8;
-    len  = quick_dist_codes[dist-1] & 0xFF;
-    quick_send_bits(s, code, len);
+    unsigned code1 = quick_len_codes[lc] >> 8;
+    unsigned len1 =  quick_len_codes[lc] & 0xFF;
+    unsigned code2 = quick_dist_codes[dist-1] >> 8;
+    unsigned len2  = quick_dist_codes[dist-1] & 0xFF;
+    quick_send_bits(s, code1, len1, code2, len2);
 }
 
 const ct_data static_ltree[L_CODES+2];
 
 static inline void static_emit_lit(deflate_state *const s, const int lit) {
-    quick_send_bits(s, static_ltree[lit].Code, static_ltree[lit].Len);
+    quick_send_bits(s, static_ltree[lit].Code, static_ltree[lit].Len, 0, 0);
     Tracecv(isgraph(lit), (stderr, " '%c' ", lit));
 }