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.
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
pngcrate 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
--releaseflag.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.