Skip to content
This repository has been archived by the owner on Sep 3, 2020. It is now read-only.

Commit

Permalink
fix madler#197, oss-fuzz/10036: only write 4 bytes per iteration in d…
Browse files Browse the repository at this point in the history
…eflate_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
  | madler#1 0x5b5752 in deflate_quick zlib-ng/arch/x86/deflate_quick.c:243:21
  | madler#2 0x590a15 in zng_deflate zlib-ng/deflate.c:952:18
  | madler#3 0x587165 in zng_compress2 zlib-ng/compress.c:59:15
  | madler#4 0x5866d3 in check_compress_level zlib-ng/test/fuzz/compress_fuzzer.c:22:3
  | madler#5 0x5862d8 in LLVMFuzzerTestOneInput zlib-ng/test/fuzz/compress_fuzzer.c:74:3
  | madler#6 0x4e9b48 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/libfuzzer/FuzzerLoop.cpp:575:15
  | madler#7 0x4a2f66 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/libfuzzer/FuzzerDriver.cpp:280:6
  | madler#8 0x4b3adb in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/libfuzzer/FuzzerDriver.cpp:715:9
  | madler#9 0x4a2091 in main /src/libfuzzer/FuzzerMain.cpp:20:10
  | madler#10 0x7fb8919b082f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/libc-start.c:291
  | madler#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)
  • Loading branch information
Sebastian Pop authored and Dead2 committed Sep 17, 2018
1 parent a970eb9 commit 11989c9
Showing 1 changed file with 30 additions and 20 deletions.
50 changes: 30 additions & 20 deletions arch/x86/deflate_quick.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

Expand Down

0 comments on commit 11989c9

Please sign in to comment.