Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify that fuzz targets are not hampered by checksums #125

Open
Shnatsel opened this issue Jun 20, 2018 · 4 comments
Open

Verify that fuzz targets are not hampered by checksums #125

Shnatsel opened this issue Jun 20, 2018 · 4 comments

Comments

@Shnatsel
Copy link
Member

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 Shnatsel changed the title lewton target doesn't bypass CRC, which prevents fuzzing Verify that fuzz targets are not hampered by checksums Jun 22, 2018
@Shnatsel
Copy link
Member Author

I have removed checksum verification in 3 crates via conditional compilation:
Shnatsel/ogg@818731b
Shnatsel/image-png@1a5c786
Shnatsel/lodepng-rust@22c87dc

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.

bors bot added a commit that referenced this issue Jul 8, 2018
126: Fix fuzz targets r=frewsxcv a=neosilky

I wrongly added `.unwrap()` to some of the fuzz targets I committed a while back. They would produce false positives so I removed them.

Thanks to @Shnatsel in #125 for pointing this out.

Co-authored-by: Daniel Lockyer <[email protected]>
@killercup
Copy link
Member

@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
Copy link
Member Author

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

@Shnatsel
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants