search

rust-fuzz / targets

🎯 A collection of fuzzing targets written in Rust.
Creative Commons Zero v1.0 Universal
105 stars 21 forks source link

Verify that fuzz targets are not hampered by checksums #125

Open Shnatsel opened 6 years ago

Shnatsel commented 6 years ago

Fuzzing lewton goes through the "vorbis inside ogg" codepath, which verifies CRC32 checksum on the input. This seems to prevent any kind of meaningful fuzzing.

I have disabled CRC32 checks in ogg crate during fuzzing (using conditional compilation as described in honggfuzz-rs readme) and immediately got panics on out-of-bounds access to a slice. It seems that because of CRC32 lewton was never really fuzzed.

Steps to reproduce the crash: git clone https://github.com/Shnatsel/lewton-fuzz, download files http://rpg.hamsterrepublic.com/ohrrpgce/File:Slash8-Bit.ogg and https://commons.wikimedia.org/wiki/File:Example.ogg put them in honggfuzz input directory, run cargo hfuzz run lewton-fuzz

I had to separate the lewton fuzz target into its own repo because I'm on stable and this repo requires nightly to build. The Cargo.toml files in its repo is repointed to the patched lewton crate that refers to patched ogg crate that doesn't perform CRC32 check in fuzzing mode. I also had to drop all .unwrap()s in the fuzz target code because they were causing panics that were false positives (duh). I'm not sure why they were there in the first place.

This problem with checksums is likely not exclusive to lewton; most multimedia formats employ checksums of some description, and this prevents unmodified libraries from being fuzzed. This needs to be verified for every fuzz target.

Shnatsel commented 6 years ago

I have removed checksum verification in 3 crates via conditional compilation: https://github.com/Shnatsel/ogg/commit/818731bcf8ebb99537fe36459f5621964bf4eba6 https://github.com/Shnatsel/image-png/commit/1a5c7866eb3d017dcea40ec6f4200bcb89d90c50 https://github.com/Shnatsel/lodepng-rust/commit/22c87dcd99ffbbe78cb85dc9e7f5d90350901892

After that fuzzers have immediately uncovered bugs in these libraries.

Is this the right approach to fuzzing formats with checksums? If so, I will open pull requests with these changes to the crates themselves.

Also, checksums should be mentioned in fuzzing documentation.

killercup commented 6 years ago

@Shnatsel I've really enjoyed your post on reddit. Do you want to continue to work on this? I'd love to add you to the rust-fuzz orga!

Shnatsel commented 6 years ago

I will be preoccupied by other things in the coming month, but after that - sure, why not.

Shnatsel commented 6 years ago

Also, I've verified that all the fuzz targets in the repo are not hampered by checksums (as far as format specifications for them go, anyway; I didn't read all of the code).

This just leaves documenting that checksums should be conditionally disabled in quickstart guides.