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

Memory Allocation Failed in image::DynamicImage::resize_exact #2340

Closed
rfuzz opened this issue Oct 8, 2024 · 3 comments · Fixed by #2342
Closed

Memory Allocation Failed in image::DynamicImage::resize_exact #2340

rfuzz opened this issue Oct 8, 2024 · 3 comments · Fixed by #2342

Comments

@rfuzz
Copy link

rfuzz commented Oct 8, 2024

While performing fuzz testing on the image::DynamicImage::resize_exact function, I discovered an input that can cause a program crash.

Expected

The program should catch this crash-inducing condition and return an error instead of crashing.

Actual behaviour

Crash with memory allocation failed.

Reproduction steps

use base64::prelude::*;

fn main() {
    if let Ok(data) = BASE64_STANDARD.decode("ZmFyYmZlbGT/UMnJAAAAAAAAAAAAANj/Nw8A") {
        match image::load_from_memory(&data) {
            Ok(img) => {
                let _ = img.resize_exact(1, 1, image::imageops::FilterType::Nearest);
            },
            Err(e) => println!("ERR: {:?}", e),
        }
    }
}

When run, the following output is produced:

❯ cargo build --release && valgrind ./target/release/image_fuzz_target_test 
==422390== Memcheck, a memory error detector
==422390== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==422390== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==422390== Command: ./target/release/image_fuzz_target_test
==422390== 
memory allocation of 68535753872 bytes failed
==422390== 
==422390== Process terminating with default action of signal 6 (SIGABRT)
==422390==    at 0x4A0F9FC: __pthread_kill_implementation (pthread_kill.c:44)
==422390==    by 0x4A0F9FC: __pthread_kill_internal (pthread_kill.c:78)
==422390==    by 0x4A0F9FC: pthread_kill@@GLIBC_2.34 (pthread_kill.c:89)
==422390==    by 0x49BB475: raise (raise.c:26)
==422390==    by 0x49A17F2: abort (abort.c:79)
==422390==    by 0x2D66B9: std::sys::pal::unix::abort_internal (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x2D1429: std::process::abort (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x2D8320: std::alloc::rust_oom (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x2D8342: __rg_oom (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x2EEB1C: alloc::alloc::handle_alloc_error (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x2EEB09: alloc::raw_vec::handle_error (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x2467F8: _ZN5image8imageops6sample15vertical_sample17h143f7a7df03255edE.llvm.1179569156878367517 (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x24990A: image::imageops::sample::resize (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390==    by 0x1ABC02: image::dynimage::DynamicImage::resize_exact (in /home/ubuntu/image_fuzz_target_test/target/release/image_fuzz_target_test)
==422390== 
==422390== HEAP SUMMARY:
==422390==     in use at exit: 115 bytes in 3 blocks
==422390==   total heap usage: 11 allocs, 8 frees, 2,235 bytes allocated

version:

[dependencies]
image = "0.25.2"
base64 = "0.22.1"
@fintelia
Copy link
Contributor

fintelia commented Oct 8, 2024

I no longer investigate fuzzing reports against parts of the API surface that haven't been hardened against untrusted input. But if you want to submit a PR, feel free!

@okaneco
Copy link
Contributor

okaneco commented Oct 9, 2024

This doesn't need to be obfuscated as base64.

Investigated on the playground, it's a farbfeld image with a 0 dimension.

use image;
use image::GenericImageView;

fn main() {
    // use base64; // 0.22.1
    // use base64::prelude::*;
    // let s = BASE64_STANDARD.decode("ZmFyYmZlbGT/UMnJAAAAAAAAAAAAANj/Nw8A").unwrap();
    // farbfeld\xffP\xc9\xc9\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\xd8\xff7\x0f\x00
    let s = [
        102u8, 97, 114, 98, 102, 101, 108, 100, 255, 80, 201, 201, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
        216, 255, 55, 15, 0,
    ];
    let im = image::load_from_memory(&s).unwrap();
    dbg!(im.dimensions());
    // [src/main.rs:15:5] im.dimensions() = (
    //     4283484617,
    //     0,
    // )
}

It aborts when you try to resize_exact like in the report.

Exited with signal 6 (SIGABRT): abort program

Guessing it's here around L473 where the width is multiplied by the height.

fn vertical_sample<I, P, S>(image: &I, new_height: u32, filter: &mut Filter) -> Rgba32FImage
where
I: GenericImageView<Pixel = P>,
P: Pixel<Subpixel = S> + 'static,
S: Primitive + 'static,
{
let (width, height) = image.dimensions();
let mut out = ImageBuffer::new(width, new_height);
let mut ws = Vec::new();

But it might've failed before this if original image had a 1px height. Not sure if ImageBuffer should support 0-sized dimensions, I think that'd been mentioned before.

@HeroicKatora
Copy link
Member

The image buffer in the base64 ends up having size: (4283484617, 0). It only includes no pixels. The vertical sampling is done first and will immediately attempt to allocate a (4283484617, 1) buffer. A quick fix might be to have vertical_sample quickly return a fresh (0, 0) buffer if no pixels had been present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants