"Index out of range" panic on decoding GIF files (both valid and malformed)

[Shnatsel]

This happens in image from git, revision 092db1e

Expected

Image decodes successfully or returns error, but doesn't panic.

Actual behaviour

When decoding the AFL-generated seed set for libgif, image panics on many of the images:

15141:thread 'main' panicked at 'index 724992 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15142-Triggered by: "./afl_testcases/gif_im/full/images/id:000477,src:000000,op:havoc,rep:16.gif"
--
15193:thread 'main' panicked at 'index 4096 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15194-Triggered by: "./afl_testcases/gif_im/full/images/id:001432,src:000003+001096,op:splice,rep:2.gif"
--
15261:thread 'main' panicked at 'index 8 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15262-Triggered by: "./afl_testcases/gif_im/full/images/id:001950,src:000962,op:havoc,rep:4.gif"
--
15277:thread 'main' panicked at 'index 4096 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15278-Triggered by: "./afl_testcases/gif_im/full/images/id:001430,src:000003+001096,op:splice,rep:16.gif"
--
15303:thread 'main' panicked at 'index 4176 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15304-Triggered by: "./afl_testcases/gif_im/full/images/id:000813,src:000481,op:arith8,pos:38,val:-12.gif"
--
15311:thread 'main' panicked at 'index 4096 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15312-Triggered by: "./afl_testcases/gif_im/full/images/id:001434,src:000003+001096,op:splice,rep:16.gif"
--
15317:thread 'main' panicked at 'index 4227136 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15318-Triggered by: "./afl_testcases/gif_im/full/images/id:000517,src:000037,op:havoc,rep:2.gif"
--
15495:thread 'main' panicked at 'index 6604900 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15496-Triggered by: "./afl_testcases/gif_im/full/images/id:000428,src:000000,op:havoc,rep:4.gif"
--
15505:thread 'main' panicked at 'index 8327168 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15506-Triggered by: "./afl_testcases/gif_im/full/images/id:000397,src:000000,op:havoc,rep:16.gif"
--
15553:thread 'main' panicked at 'index 4227136 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15554-Triggered by: "./afl_testcases/gif_im/full/images/id:000572,src:000080,op:havoc,rep:2.gif"
--
15599:thread 'main' panicked at 'index 8 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15600-Triggered by: "./afl_testcases/gif_im/full/images/id:001971,src:001950,op:havoc,rep:4.gif"
15601:thread 'main' panicked at 'index 1024 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15602-Triggered by: "./afl_testcases/gif_im/full/images/id:000686,src:000131,op:havoc,rep:16.gif"
--
15625:thread 'main' panicked at 'index 4096 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15626-Triggered by: "./afl_testcases/gif_im/full/images/id:001720,src:001434,op:flip1,pos:43.gif"
--
15657:thread 'main' panicked at 'index 4096 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15658-Triggered by: "./afl_testcases/gif_im/full/images/id:001429,src:000003+001096,op:splice,rep:32.gif"
--
15675:thread 'main' panicked at 'index 128 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15676-Triggered by: "./afl_testcases/gif_im/full/images/id:000589,src:000080,op:havoc,rep:8.gif"
--
15713:thread 'main' panicked at 'index 1024 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15714-Triggered by: "./afl_testcases/gif_im/full/images/id:001139,src:000679,op:flip1,pos:6.gif"
--
15819:thread 'main' panicked at 'index 4096 out of range for slice of length 0', src/libcore/slice/mod.rs:2725:5
15820-Triggered by: "./afl_testcases/gif_im/full/images/id:001433,src:000003+001096,op:splice,rep:2.gif"
--
15839:thread 'main' panicked at 'index 724992 out of range for slice of length 4096', src/libcore/slice/mod.rs:2725:5
15840-Triggered by: "./afl_testcases/gif_im/edges-only/images/id:000477,src:000000,op:havoc,rep:16.gif"

Reproduction steps

panicking_afl_gif_testcases.tar.gz

fn main() -> std::io::Result<()> {
    let path = std::env::args().skip(1).next().unwrap();
    let image = image::io::Reader::open(path)?.guess_format().decode().unwrap();
    Ok(())
}

[Shnatsel]

This would be easily discovered by a fuzzer, FWIW.

[Veykril]

The problem seems to be that the image buffer passed to read_image is too small for the gif decoder to work with causing an out of bounds panic in https://github.com/image-rs/image-gif/blob/9c0e0965afb8186492ef6a3a73215d40f7ab8a96/src/reader/mod.rs#L250. The problem here is that the gif reader's Reader::buffer_size function returns a different value than what the ImageDecoder::total_bytes function returns but simply adjusting the ImageDecoder::total_bytes function to make use of the readers buffer size doesn't work as the resulting image buffer Vec would have a different size from what most of the internal image API expects regarding its size being width * height.

[HeroicKatora]

There are two different dimensions at play:

• self.current_frame.width and
• self.decoder.decoder.width()

Similar thing happens in apng, where each frame can specific its own dimensions and position within the complete image such that not every part of the screen must be updated as well. The image::AnimationDecoder assumes, as well as consumers such as emulasion, that all images yielded by it have the same dimensions. Maybe the validation that the dimensions are in fact smaller is missing here.

[HeroicKatora]

It seems to have no validation: https://github.com/image-rs/image-gif/blob/master/src/reader/decoder.rs#L333

No on to the spec to find if that is indeed forbidden.

[fseegraeber]

The spec says: "Each image must fit within the boundaries of the Logical Screen [...]". Where image is a frame, and the "Logical Screen" basically the overall gif image.

The decoder allows checking for that with check_frame_consistency (is off by default):
https://github.com/image-rs/image-gif/blob/5b53dc1b61967473d13707be5ab62e57e82d92d4/src/reader/mod.rs#L87-L88

A potential fix seems easy enough.