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

Overflow when decoding image file #118

Open
qarmin opened this issue Oct 22, 2024 · 3 comments
Open

Overflow when decoding image file #118

qarmin opened this issue Oct 22, 2024 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@qarmin
Copy link

qarmin commented Oct 22, 2024

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

@fintelia
Copy link
Contributor

Want to submit a PR?

@qarmin
Copy link
Author

qarmin commented Oct 23, 2024

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

@HeroicKatora HeroicKatora added the help wanted Extra attention is needed label Oct 23, 2024
@fintelia
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants