Skip to content

Commit

Permalink
Add fuzz target for decompression and fix invalid code len bug
Browse files Browse the repository at this point in the history
Fuzz test is only for non-wrapping buffers for now
Some invalid data resulted in 0-length codes being decoded, which
resulted in an infinite loop causing OOM.
  • Loading branch information
oyvindln committed Jan 28, 2018
1 parent 0adc62b commit b53177a
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 60 deletions.
26 changes: 0 additions & 26 deletions Cargo_fuzz.toml

This file was deleted.

6 changes: 6 additions & 0 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ libc="0.2.22"

[dependencies.miniz_oxide_c_api]
path = ".."
[dependencies.miniz_oxide]
path = "../miniz_oxide"
[dependencies.libfuzzer-sys]
git = "https://github.com/rust-fuzz/libfuzzer-sys.git"

Expand All @@ -28,3 +30,7 @@ path = "fuzz_targets/fuzz_high.rs"
default = ["fuzzing"]
fuzzing = ["miniz_oxide_c_api/fuzzing"]


[[bin]]
name = "inflate_nonwrapping"
path = "fuzz_targets/inflate_nonwrapping.rs"
Empty file modified fuzz/fuzz_targets/fuzz_high.rs
100644 → 100755
Empty file.
8 changes: 8 additions & 0 deletions fuzz/fuzz_targets/inflate_nonwrapping.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![no_main]
#[macro_use] extern crate libfuzzer_sys;
extern crate miniz_oxide;

fuzz_target!(|data: &[u8]| {
// fuzzed code goes here
let _ = miniz_oxide::inflate::decompress_to_vec(data);
});
94 changes: 60 additions & 34 deletions miniz_oxide/src/inflate/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,24 @@ impl HuffmanTable {

#[inline]
/// Look up a symbol and code length from the bits in the provided bit buffer.
fn lookup(&self, bit_buf: BitBuffer) -> (i32, u32) {
///
/// Returns Some(symbol, length) on success,
/// None if the length is 0.
///
/// It's possible we could avoid checking for 0 if we can guarantee a sane table.
/// TODO: Check if a smaller type for code_len helps performance.
fn lookup(&self, bit_buf: BitBuffer) -> Option<(i32, u32)> {
let symbol = self.fast_lookup(bit_buf).into();
if symbol >= 0 {
(symbol, (symbol >> 9) as u32)
if (symbol >> 9) as u32 != 0 {
Some((symbol, (symbol >> 9) as u32))
} else {
// Zero-length code.
None
}
} else {
// We didn't get a symbol from the fast lookup table, so check the tree instead.
self.tree_lookup(symbol.into(), bit_buf, FAST_LOOKUP_BITS.into())
Some(self.tree_lookup(symbol.into(), bit_buf, FAST_LOOKUP_BITS.into()))
}
}
}
Expand Down Expand Up @@ -253,6 +264,7 @@ enum State {
BadCodeSizeDistPrevLookup,
InvalidLitlen,
InvalidDist,
InvalidCodeLen,
}

impl State {
Expand Down Expand Up @@ -779,7 +791,7 @@ fn decompress_fast(

fill_bit_buffer(&mut l, &mut in_iter);

let (symbol, code_len) = r.tables[LITLEN_TABLE].lookup(l.bit_buf);
if let Some((symbol, code_len)) = r.tables[LITLEN_TABLE].lookup(l.bit_buf) {

l.counter = symbol as u32;
l.bit_buf >>= code_len;
Expand All @@ -795,22 +807,29 @@ fn decompress_fast(
fill_bit_buffer(&mut l, &mut in_iter);
}

let (symbol, code_len) = r.tables[LITLEN_TABLE].lookup(l.bit_buf);

l.bit_buf >>= code_len;
l.num_bits -= code_len;
// The previous symbol was a literal, so write it directly and check
// the next one.
out_buf.write_byte(l.counter as u8);
if (symbol & 256) != 0 {
l.counter = symbol as u32;
// The symbol is a length value.
break;
if let Some((symbol, code_len)) = r.tables[LITLEN_TABLE].lookup(l.bit_buf) {
l.bit_buf >>= code_len;
l.num_bits -= code_len;
// The previous symbol was a literal, so write it directly and check
// the next one.
out_buf.write_byte(l.counter as u8);
if (symbol & 256) != 0 {
l.counter = symbol as u32;
// The symbol is a length value.
break;
} else {
// The symbol is a literal, so write it directly and continue.
out_buf.write_byte(symbol as u8);
}
} else {
// The symbol is a literal, so write it directly and continue.
out_buf.write_byte(symbol as u8);
state.begin(InvalidCodeLen);
break 'o TINFLStatus::Failed;
}
}
} else {
state.begin(InvalidCodeLen);
break 'o TINFLStatus::Failed;
}
}

state.begin(HuffDecodeOuterLoop1);
Expand Down Expand Up @@ -1372,7 +1391,7 @@ fn decompress_inner(
} else {
fill_bit_buffer(&mut l, &mut in_iter);

let (symbol, code_len) = r.tables[LITLEN_TABLE].lookup(l.bit_buf);
if let Some((symbol, code_len)) = r.tables[LITLEN_TABLE].lookup(l.bit_buf) {

l.counter = symbol as u32;
l.bit_buf >>= code_len;
Expand All @@ -1388,23 +1407,29 @@ fn decompress_inner(
fill_bit_buffer(&mut l, &mut in_iter);
}

let (symbol, code_len) = r.tables[LITLEN_TABLE].lookup(l.bit_buf);

l.bit_buf >>= code_len;
l.num_bits -= code_len;
// The previous symbol was a literal, so write it directly and check
// the next one.
out_buf.write_byte(l.counter as u8);
if (symbol & 256) != 0 {
l.counter = symbol as u32;
// The symbol is a length value.
Action::Jump(HuffDecodeOuterLoop1)
if let Some((symbol, code_len)) = r.tables[LITLEN_TABLE].lookup(l.bit_buf) {

l.bit_buf >>= code_len;
l.num_bits -= code_len;
// The previous symbol was a literal, so write it directly and check
// the next one.
out_buf.write_byte(l.counter as u8);
if (symbol & 256) != 0 {
l.counter = symbol as u32;
// The symbol is a length value.
Action::Jump(HuffDecodeOuterLoop1)
} else {
// The symbol is a literal, so write it directly and continue.
out_buf.write_byte(symbol as u8);
Action::None
}
} else {
// The symbol is a literal, so write it directly and continue.
out_buf.write_byte(symbol as u8);
Action::None
Action::Jump(InvalidCodeLen)
}
}
} else {
Action::Jump(InvalidCodeLen)
}
}
})
}
Expand Down Expand Up @@ -1651,7 +1676,8 @@ fn decompress_inner(

// Anything else indicates failure.
// BadZlibHeader | BadRawLength | BlockTypeUnexpected | DistanceOutOfBounds |
// BadTotalSymbols | BadCodeSizeDistPrevLookup | BadCodeSizeSum
// BadTotalSymbols | BadCodeSizeDistPrevLookup | BadCodeSizeSum | InvalidLitlen |
// InvalidDist | InvalidCodeLen
_ => break TINFLStatus::Failed,
};
};
Expand Down Expand Up @@ -1774,7 +1800,7 @@ mod test {
}

fn masked_lookup(table: &HuffmanTable, bit_buf: BitBuffer) -> (i32, u32) {
let ret = table.lookup(bit_buf);
let ret = table.lookup(bit_buf).unwrap();
(ret.0 & 511, ret.1)
}

Expand Down
6 changes: 6 additions & 0 deletions miniz_oxide/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ fn inf_issue_19() {
let _ = decompress_to_vec(data.as_slice());
}

#[test]
fn decompress_oom() {
let data = get_test_file_data("tests/test_data/invalid_code_len_oom");
let _ = decompress_to_vec(data.as_slice());
}

fn get_test_data() -> Vec<u8> {
use std::env;
let path = env::var("TEST_FILE").unwrap_or_else(|_| "../miniz/miniz.c".to_string());
Expand Down
Binary file added miniz_oxide/tests/test_data/invalid_code_len_oom
Binary file not shown.

0 comments on commit b53177a

Please sign in to comment.