diff --git a/Cargo_fuzz.toml b/Cargo_fuzz.toml deleted file mode 100644 index a9af3827..00000000 --- a/Cargo_fuzz.toml +++ /dev/null @@ -1,26 +0,0 @@ -[package] -name = "miniz_oxide" -version = "0.1.0" -build = "src/build.rs" -links = "miniz" -license = "MIT" -authors = ["Frommi "] - -[workspace] - -[lib] -name = "miniz_oxide" -crate-type = ['rlib'] - -[dependencies] -libc="0.2.22" - -[build-dependencies] -gcc = "0.3" - -[features] -default = [] -fuzzing = [] - -[profile.dev] -panic = "abort" diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index f1c51c06..39667d57 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -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" @@ -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" diff --git a/fuzz/fuzz_targets/fuzz_high.rs b/fuzz/fuzz_targets/fuzz_high.rs old mode 100644 new mode 100755 diff --git a/fuzz/fuzz_targets/inflate_nonwrapping.rs b/fuzz/fuzz_targets/inflate_nonwrapping.rs new file mode 100755 index 00000000..38f9fd59 --- /dev/null +++ b/fuzz/fuzz_targets/inflate_nonwrapping.rs @@ -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); +}); diff --git a/miniz_oxide/src/inflate/core.rs b/miniz_oxide/src/inflate/core.rs index 6978c300..b78b5313 100644 --- a/miniz_oxide/src/inflate/core.rs +++ b/miniz_oxide/src/inflate/core.rs @@ -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())) } } } @@ -253,6 +264,7 @@ enum State { BadCodeSizeDistPrevLookup, InvalidLitlen, InvalidDist, + InvalidCodeLen, } impl State { @@ -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; @@ -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); @@ -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; @@ -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) + } } }) } @@ -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, }; }; @@ -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) } diff --git a/miniz_oxide/tests/test.rs b/miniz_oxide/tests/test.rs index 5549c85d..5cd5796e 100644 --- a/miniz_oxide/tests/test.rs +++ b/miniz_oxide/tests/test.rs @@ -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 { use std::env; let path = env::var("TEST_FILE").unwrap_or_else(|_| "../miniz/miniz.c".to_string()); diff --git a/miniz_oxide/tests/test_data/invalid_code_len_oom b/miniz_oxide/tests/test_data/invalid_code_len_oom new file mode 100644 index 00000000..3e030474 Binary files /dev/null and b/miniz_oxide/tests/test_data/invalid_code_len_oom differ