image-webp icon indicating copy to clipboard operation
image-webp copied to clipboard

Overflow when decoding image file

Open qarmin opened this issue 1 year ago • 3 comments

code

fn check_file(file_path: &str) {
    let Ok(content) = fs::read(file_path) else {
        return;
    };
    let res = match image::load_from_memory(&content) {
        Ok(res) => res,
        Err(e) => {
            eprintln!("Error: {}", e);
            return;
        }
    };
    println!("Image: {file_path}");
    for format in [
        ImageFormat::Bmp,
        ImageFormat::Farbfeld,
        ImageFormat::Ico,
        ImageFormat::Jpeg,
        ImageFormat::Png,
        ImageFormat::Pnm,
        ImageFormat::Tiff,
        ImageFormat::WebP,
        ImageFormat::Tga,
        ImageFormat::Dds,
        ImageFormat::Hdr,
        ImageFormat::OpenExr,
        // ImageFormat::Avif, // Don't use, it is really slow https://github.com/image-rs/image/issues/2282
        ImageFormat::Qoi,
    ]
        .into_iter()
    {
        let buffer: Vec<u8> = Vec::new();
        println!("Before write_to {format:?}");
        if let Err(e) = res.write_to(&mut Cursor::new(buffer), format) {
            eprintln!("Error: {}", e);
        };
        println!("After write_to {format:?}");
    }
}

causes this panic

thread 'main' panicked at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/huffman.rs:66:25:
attempt to add with overflow
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Aborted (core dumped)
rafal@rafal-komputer:~/Downloads/REPORTS___IMAGE/image_rs_overflow_a__(3172 bytes) - 15904117247202565519$ ^C
rafal@rafal-komputer:~/Downloads/REPORTS___IMAGE/image_rs_overflow_a__(3172 bytes) - 15904117247202565519$ RUST_BACKTRACE=1 image output
thread 'main' panicked at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/huffman.rs:66:25:
attempt to add with overflow
stack backtrace:
   0: rust_begin_unwind
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/std/src/panicking.rs:665:5
   1: core::panicking::panic_fmt
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/panicking.rs:74:14
   2: core::panicking::panic_const::panic_const_add_overflow
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/panicking.rs:181:21
   3: image_webp::huffman::HuffmanTree::build_implicit
             at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/huffman.rs:66:25
   4: image_webp::lossless::LosslessDecoder<R>::read_huffman_code
             at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/lossless.rs:390:13
   5: image_webp::lossless::LosslessDecoder<R>::read_huffman_codes
             at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/lossless.rs:330:28
   6: image_webp::lossless::LosslessDecoder<R>::decode_image_stream
             at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/lossless.rs:184:28
   7: image_webp::lossless::LosslessDecoder<R>::decode_frame
             at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/lossless.rs:121:9
   8: image_webp::decoder::WebPDecoder<R>::read_image
             at /home/rafal/.cargo/registry/src/index.crates.io-6f17d22bba15001f/image-webp-0.2.0/src/decoder.rs:629:17
   9: <image::codecs::webp::decoder::WebPDecoder<R> as image::image::ImageDecoder>::read_image
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/codecs/webp/decoder.rs:55:9
  10: <image::codecs::webp::decoder::WebPDecoder<R> as image::image::ImageDecoder>::read_image_boxed
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/codecs/webp/decoder.rs:61:9
  11: <alloc::boxed::Box<T> as image::image::ImageDecoder>::read_image
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/image.rs:743:9
  12: image::image::decoder_to_vec
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/image.rs:608:5
  13: image::dynimage::decoder_to_image
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/dynimage.rs:1189:23
  14: image::dynimage::DynamicImage::from_decoder
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/dynimage.rs:222:9
  15: image::image_reader::image_reader_type::ImageReader<R>::decode
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/image_reader/image_reader_type.rs:271:9
  16: image::image_reader::free_functions::load
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/image_reader/free_functions.rs:23:5
  17: image::dynimage::load_from_memory_with_format
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/dynimage.rs:1352:5
  18: image::dynimage::load_from_memory
             at /home/rafal/.cargo/git/checkouts/image-c1c0bf49fde10069/5e6bf4f/src/dynimage.rs:1338:5
  19: image::check_file
             at /home/rafal/Projekty/Rust/run_command_for_every_file/src/crates/image/src/main.rs:32:21
  20: image::main
             at /home/rafal/Projekty/Rust/run_command_for_every_file/src/crates/image/src/main.rs:24:9
  21: core::ops::function::FnOnce::call_once
             at /rustc/eeb90cda1969383f56a2637cbd3037bdf598841c/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

File - output.zip

qarmin avatar Oct 22 '24 18:10 qarmin

Want to submit a PR?

fintelia avatar Oct 23 '24 02:10 fintelia

Not really, because I don't really know what fix should be applied(clamping?, wrapping? or maybe completely different behavior)

qarmin avatar Oct 23 '24 04:10 qarmin

Figuring out the proper fix is a big part of the work of fuzzing.

In this case, the surrounding code is computing a sum and then checking whether it exactly equals the expected value. So the proper thing is to either use saturating or checked arithmetic to ensure that the function always returns an error on overflow

fintelia avatar Oct 30 '24 17:10 fintelia