-
Notifications
You must be signed in to change notification settings - Fork 145
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
Unbounded memory consumption on malformed inputs #80
Comments
Shnatsel
added a commit
to Shnatsel/image-png
that referenced
this issue
Jun 27, 2018
Closed
It is also possible that the overflow is not to blame, and is merely a side effect of incorrect handling of malformed files. |
Shnatsel
added a commit
to Shnatsel/image-png
that referenced
this issue
Jun 27, 2018
This was referenced Jun 27, 2018
I think good solution will be to add |
zealousidealroll
pushed a commit
to zealousidealroll/image-png
that referenced
this issue
Sep 1, 2018
zealousidealroll
added a commit
to zealousidealroll/image-png
that referenced
this issue
Sep 6, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There is an integer overflow in https://github.com/PistonDevelopers/image-png/blob/99383650e1a440bb14c54987938676c8f54d3bc6/src/decoder/mod.rs#L51
Aside of posing dangers for unsafe code (which shouldn't rely on this value anyway), this overflow causes enormous amounts of memory to be actually allocated when fed to
png
crate via the fuzzing harness. Not just virtual memory - actual physical memory.The worst part is, fixing this correctly requires changing the external API: the function should use checked_mul() which returns
Option<usize>
, and actually return either an Option or Result to the outside.Testcase: integer_overflow_in_multiplication found via afl-rs. Steps to reproduce the crash can be found in #79 except you need to build in debug mode, without the
--release
flag.I would appreciate advice on how to proceed with fixing this issue. Is adding "deprecated" marker to this function in 0.12 series and releasing 0.13 with a breaking fix appropriate? Do we need the semver trick here?
Update: libpng itself also had similar issues; see https://libpng.sourceforge.io/decompression_bombs.html for more info. Among other things, they have introduced limits on the possible size of an image by default. In Rust we can easily allow the API user to override these limits via the builder pattern.
The text was updated successfully, but these errors were encountered: