image-rs
OOM in ICO decoding (found by fuzzer)
Decoding some ICO files causes image to allocate an unbounded amount of memory, which the fuzzer registers as an OOM and aborts. This could conceivably allow for DoS, but my immediate concern is that it blocks further fuzzing.
This happens on commit 04052e64c9a94606efc8bd3d87d5f3e0f566774e (latest as of this writing)
Reproduction steps
cargo +nightly fuzz run fuzzer_script_ico path/to/file
Files triggering the issue: ico-ooms.tar.gz
Relevant section from the backtrace of oom-ff076e7064c13ec1f8e2c5bbbfb7a77a08db2361:
#9 0x5585de96da97 in alloc::raw_vec::RawVec$LT$T$C$A$GT$::reserve::do_reserve_and_handle::h328250443c453bbe (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x115ca97) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#10 0x5585debf9ee9 in png::decoder::Reader$LT$R$GT$::allocate_out_buf::hc36015c795f777d2 (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x13e8ee9) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#11 0x5585debffdf5 in png::decoder::Reader$LT$R$GT$::init::h0063be5cb77ca9e0 (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x13eedf5) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#12 0x5585dec0273e in png::decoder::Decoder$LT$R$GT$::read_info::hc3c03aa577d25dc3 (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x13f173e) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#13 0x5585dee6ce75 in image::codecs::png::PngDecoder$LT$R$GT$::new::h8c36bf137bb1a79d (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x165be75) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#14 0x5585dee6c5dc in image::codecs::ico::decoder::DirEntry::decoder::h67893ff245f0ca90 (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x165b5dc) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#15 0x5585dee6b690 in image::codecs::ico::decoder::IcoDecoder$LT$R$GT$::new::h343ef4b0a7af1775 (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x165a690) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
#16 0x5585dee0d306 in image::io::free_functions::load_decoder::h2dc7d85cf608012b (/home/jonathan/git/image/fuzz/target/x86_64-unknown-linux-gnu/release/fuzzer_script_ico+0x15fc306) (BuildId: f477f2e66325228dc2848ee25476a45eea972b96)
In more detail, what is happening is the ICO image contains an embedded PNG. That PNG has bogus (and giant) values for width and/or height. In the image crate, limits are set via the IcoDecoder::set_limits method which gets called after IcoDecoder::new. However, within the IcoDecoder::new method we need to scan the metadata of the embedded PNG, which we do using the png crate. Now png requires us to pass a memory limit, but it isn't known yet, so we pass usize::max_value and then subsequently crash when it allocates an output buffer during init that is based on the bogus with and height.
Unfortunately, I don't see a quick way of fixing this that avoids doing a bunch of interface changes and restructuring.
We should fix png to allow reconfiguring of limits after construction. We shouldn't require more than the default amount for intents of reading the header chunks, so this should be in-time.
