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

Unbounded memory consumption on malformed inputs #80

Closed
Shnatsel opened this issue Jun 27, 2018 · 2 comments
Closed

Unbounded memory consumption on malformed inputs #80

Shnatsel opened this issue Jun 27, 2018 · 2 comments

Comments

@Shnatsel
Copy link
Contributor

Shnatsel commented Jun 27, 2018

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.

Shnatsel added a commit to Shnatsel/image-png that referenced this issue Jun 27, 2018
@Shnatsel Shnatsel changed the title Integer overflow in buffer_size() Huge memory consumption on malformed inputs Jun 27, 2018
@Shnatsel
Copy link
Contributor Author

It is also possible that the overflow is not to blame, and is merely a side effect of incorrect handling of malformed files.

@Shnatsel Shnatsel changed the title Huge memory consumption on malformed inputs Unbounded memory consumption on malformed inputs Jun 27, 2018
Shnatsel added a commit to Shnatsel/image-png that referenced this issue Jun 27, 2018
@newpavlov
Copy link

I think good solution will be to add Limits struct and check against it, as for example was done in flif crate

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants