search

Frommi / miniz_oxide

Rust replacement for miniz
MIT License
174 stars 49 forks source link

adler32 check in fuzz target interferes with fuzzing #29

Closed Shnatsel closed 6 years ago

Shnatsel commented 6 years ago

The fuzz target currently invokes miniz_oxide like this: https://github.com/Frommi/miniz_oxide/blob/01a57125fc0c6ba87629adfa7b3046030f2f347b/fuzz/fuzz_targets/inflate_nonwrapping.rs#L7

It does not seem to disable alder32 checks. Because of checksum verification fuzzers cannot reach the actually interesting decoding code.

Rust fuzzer integration provides special configuration option fuzzing which is set when a binary is built with fuzzing instrumentation; you can use it to disable checksums via conditional compilation (example from lodepng-rust). Alternatively, you can adapt your fuzzing target to set the required configuration parameters to disable checksum verification (inflate crate provides a *_no_checksum function).

cc @oyvindln who has authored the fuzz target.

I can also provide you with an initial fuzzing corpus to kickstart fuzzing, both with and without zlib headers, obtained by fuzzing inflate crate with afl-fuzz, libfuzzer and honggfuzz. If you're interested, let me know and I'll attach it here.

honggfuzz-rs provides further documentation, including enabling sanitizers to catch bugs in code under unsafe. Or ping me or https://github.com/rust-fuzz if you have any questions regarding fuzzing.

oyvindln commented 6 years ago

Checksum validation happens at the very end, after decoding, so I'm not sure how that would be blocking anything. Could probably add a flag to disable it though, more fuzzing coverage won't hurt in any case. Didn't do much with the fuzzing code for a long time due to a rustc bug that triggered an LLVM assertion, but that's been fixed now. What's there currently is very basic, so adding your fuzzing corpus sounds like a good idea. We should be fuzzing streaming decoding as well.

I've been working on adding some basic coverage tests ported from zlib-ng, which I will commit soon.

Shnatsel commented 6 years ago

Here are the fuzzing seeds, with and without zlib headers respectively. You will probably want to minify them before running the fuzzing. Just don't forget to add these to your fuzzing set and minify again after adding any new features.

min_starting_points.tar.gz nozlib_min_starting_points.tar.gz

oyvindln commented 6 years ago

Thanks.

As miniz_oxide seems to have made its way into gecko, getting a proper fuzzing setup has become even more important.

Shnatsel commented 6 years ago

I've disabled checksum verification and run the existing "inflate_nonwrapping" fuzz target through a total of over 1 billion iterations under afl-fuzz, honggfuzz and cargo-fuzz. That found no panics, crashes or memory leaks, including under address sanitizer. Kudos!

I've opened a pull request to disable alder32 and add the fuzzing seeds to kickstart future fuzzing.

oyvindln commented 6 years ago

That sounds promising, nice work! Next up we should probably add some more fuzzing targets for different flags and for streaming configuration.