-
Notifications
You must be signed in to change notification settings - Fork 140
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
Fix panics on malformed inputs, support fuzzing #81
Conversation
…ode using conditional compilation. Enables fuzzers to actually reach PNG decoding code instead of never going beyond checksums
… Fixes panic on malformed input (image-rs#79)
…anic on malformed input (image-rs#79)
… the previous one. Fixes panic on malformed files (image-rs#79) and also likely fixes decoding of some exotic PNGs out there. Found via afl.rs
…g speed ~10x, among other things.
…h does pretty much the same thing a bit better. Integration with other fuzzers will be done via a generic harness in https://github.com/rust-fuzz/targets
…g a bunch of tools (libpng, lodepng-rust), then used for fuzzing image-png with afl, and the resulting corpus minified with afl-cmin. As such they provide good starting coverage for afl and can serve as seeds for more computationally expensive tools.
…g mode panic on overflow in left shift (image-rs#79)
version = "0.1.0" | ||
authors = ["nwin <nwin@users.noreply.github.com>"] | ||
version = "0.2.0" | ||
authors = ["Sergey Davidoff <shnatsel@gmail.com>", "Paul Grandperrin <paul.grandperrin@gmail.com>"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think nwin removed as author here by accident.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have entirely replaced the afl/ subfolder without using anything that's in there previously. Its current incarnation is based only on https://github.com/rust-fuzz/targets, which is why Paul Grandperrin is credited.
I agree nwin should be credited for his contribution in some way, but I have removed him from the copyright notice for the source files the current afl/ folder is not based on his work in any way, and such misattribution is deliberately prohibited under some licenses, e.g. some BSD variants. This crate is dual-licensed under MIT and Apache, which do not have such a clause, so crediting nwin here would not be in conflict with the license.
I will re-add him as an author if you believe that is the best way to go about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it's under the "png-afl" directory, didn't saw that.
@nwin OK with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I split the panic fixes to a separate PR so they would not be held up by a copyright notice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's OK. @nwin can just add himself back as author if he disagrees.
Merging. |
Bypass crc32 and adler32 checks when compiled with fuzzing instrumentation. This lets fuzzers reach actually interesting code.
Add a starting corpus for fuzzing, obtained initially by fuzzing other tools, then fed to afl-fuzz on image-png to get more inputs specific to it, and subsequently minified with afl-cmin.
Fix panics on malformed inputs discovered via fuzzing (#79):
expand_paletted()
: returnResult
instead of callingunwrap()
unfilter()
: now checks bounds and returns Resultnext_interlaced_row():
info
from previous row was erroneously used to decode the current row. Fixing this no longer triggers a panic and likely fixes decoding of some real-world PNGs.Not fixed in this PR: