From c2cdb112b702089ef7784cefa8e5105f961c7528 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 10:55:54 -0700 Subject: [PATCH] LR-IMP-FORMATS: GIF and BMP sizes from the header; a canvas the file can't fill is refused (#2553) read_header had no reader for GIF or BMP and fell back to a full decode, so a few-dozen-byte file declaring a canvas just under the 1-gigapixel limit made the import probe allocate ~4.3 GB; a failed allocation aborts the process. A decode had the same allocation. - other::header reads the size from the header. A canvas up to 4 MP is still decoded there, so truncated small files are refused at import as before. - check_fillable refuses a canvas larger than the file's bytes can describe (BMP: 128 pixels per byte, covering 1-bit and RLE; GIF: 11,000, an LZW code of at least 3 bits expanding to at most 4096 pixels), in both the probe and the decode, before the decoder allocates. - The GIF and BMP decoders get an allocation limit (the RGBA buffer of max_pixels). The approach follows the abandoned #2182 (PierrunoYT). JPEG XL still decodes in the probe. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01K93HTyCiwYvbRGaAkjPvXn --- crates/codecs/src/lib.rs | 1 + crates/codecs/src/other.rs | 56 ++++++++++++++++++++++++++++++++++ crates/codecs/tests/fuzz.rs | 61 +++++++++++++++++++++++++++++++++++++ docs/whats-new.md | 8 +++++ 4 files changed, 126 insertions(+) diff --git a/crates/codecs/src/lib.rs b/crates/codecs/src/lib.rs index a20a47d14..8210f33f0 100644 --- a/crates/codecs/src/lib.rs +++ b/crates/codecs/src/lib.rs @@ -207,6 +207,7 @@ pub fn read_header_unguarded(bytes: &[u8], format: Format) -> Result
{ Format::WebP => webp::header(bytes)?, Format::Psd => psd::header(bytes)?, Format::Heif => heif::header(bytes)?, + Format::Gif | Format::Bmp => other::header(bytes, format)?, // rare here (or, for JPEG XL, without a header reader yet): a small decode, as before _ => { let d = decode_unguarded(bytes, format, &DecodeOptions::fit(64, 64))?; diff --git a/crates/codecs/src/other.rs b/crates/codecs/src/other.rs index d5659de90..f99bd5f17 100644 --- a/crates/codecs/src/other.rs +++ b/crates/codecs/src/other.rs @@ -4,6 +4,58 @@ use crate::convert::{Buf, Meta, Model, Raw, check_size, finish}; use crate::{DecodeOptions, Decoded, Error, Format, Result}; use image::{DynamicImage, ImageDecoder}; +/// Canvases up to this many pixels are still decoded by [`header`] (at most 16 MB), so a truncated +/// small file is refused at import as before. +const HEADER_DECODE_MAX_PIXELS: u64 = 4_000_000; + +/// Stored size for the import probe (issue #2553). The probe used to decode these in full, and a +/// few-dozen-byte file declaring a gigapixel canvas allocated gigabytes. The size now comes from the +/// header and must be one the file's bytes can fill; only a canvas small enough to be cheap is +/// decoded (to catch truncation). Orientation is always 1: neither format carries EXIF here. +pub(crate) fn header(bytes: &[u8], format: Format) -> Result<(u32, u32, u16)> { + let err = |e: image::ImageError| Error::Malformed(format, e.to_string()); + let cursor = std::io::Cursor::new(bytes); + let (w, h) = match format { + Format::Gif => image::codecs::gif::GifDecoder::new(cursor).map_err(err)?.dimensions(), + Format::Bmp => image::codecs::bmp::BmpDecoder::new(cursor).map_err(err)?.dimensions(), + _ => return Err(Error::Unsupported(format, "not handled by this decoder")), + }; + if w == 0 || h == 0 { + return Err(Error::Malformed(format, "zero dimension".into())); + } + check_fillable(format, bytes.len(), w, h)?; + if u64::from(w).saturating_mul(u64::from(h)) <= HEADER_DECODE_MAX_PIXELS { + let d = decode(bytes, format, &DecodeOptions::fit(64, 64))?; + return Ok((d.source_width, d.source_height, d.orientation)); + } + Ok((w, h, 1)) +} + +/// The decoder's allocation limit: the RGBA buffer of the largest image `opts` allows, so an +/// oversized canvas is an error from the decoder instead of an allocation attempt. +fn limits(opts: &DecodeOptions) -> image::Limits { + let mut l = image::Limits::default(); + l.max_alloc = Some(opts.max_pixels.saturating_mul(4)); + l +} + +/// The most pixels `len` bytes of pixel data can describe: BMP stores at least 1 bit per pixel and +/// its RLE at most 255 pixels per 2-byte run (128 per byte covers both); a GIF LZW code takes at +/// least 3 bits and expands to at most 4096 pixels (11,000 per byte). The decoders allocate the +/// whole canvas before reading any pixel data, so a header declaring far more than the file can +/// fill is refused first (issue #2553). +fn fillable(format: Format, len: usize) -> u64 { + let per_byte = if format == Format::Gif { 11_000 } else { 128 }; + (len as u64).saturating_mul(per_byte) +} + +fn check_fillable(format: Format, len: usize, w: u32, h: u32) -> Result<()> { + if u64::from(w).saturating_mul(u64::from(h)) > fillable(format, len) { + return Err(Error::Malformed(format, format!("a {w} × {h} canvas is far larger than its {len} bytes can hold"))); + } + Ok(()) +} + pub(crate) fn decode(bytes: &[u8], format: Format, opts: &DecodeOptions) -> Result { let err = |e: image::ImageError| Error::Malformed(format, e.to_string()); let cursor = std::io::Cursor::new(bytes); @@ -12,6 +64,8 @@ pub(crate) fn decode(bytes: &[u8], format: Format, opts: &DecodeOptions) -> Resu let mut d = image::codecs::gif::GifDecoder::new(cursor).map_err(err)?; let (w, h) = d.dimensions(); check_size(format, w as u64, h as u64, opts)?; + check_fillable(format, bytes.len(), w, h)?; + d.set_limits(limits(opts)).map_err(err)?; let icc = d.icc_profile().ok().flatten(); (DynamicImage::from_decoder(d).map_err(err)?, icc) } @@ -19,6 +73,8 @@ pub(crate) fn decode(bytes: &[u8], format: Format, opts: &DecodeOptions) -> Resu let mut d = image::codecs::bmp::BmpDecoder::new(cursor).map_err(err)?; let (w, h) = d.dimensions(); check_size(format, w as u64, h as u64, opts)?; + check_fillable(format, bytes.len(), w, h)?; + d.set_limits(limits(opts)).map_err(err)?; let icc = d.icc_profile().ok().flatten(); (DynamicImage::from_decoder(d).map_err(err)?, icc) } diff --git a/crates/codecs/tests/fuzz.rs b/crates/codecs/tests/fuzz.rs index bfee84dfe..d52f80ee0 100644 --- a/crates/codecs/tests/fuzz.rs +++ b/crates/codecs/tests/fuzz.rs @@ -74,6 +74,67 @@ fn thumbnail_rejects_huge_gif_before_allocating_source_pixels() { assert!(matches!(result, Err(Error::TooLarge(42_127, 16_304))), "{result:?}"); } +/// A GIF of a few dozen bytes: a `w` × `h` logical screen and one empty frame of that size. +fn tiny_gif(w: u16, h: u16) -> Vec { + let mut b = b"GIF89a".to_vec(); + b.extend(w.to_le_bytes()); + b.extend(h.to_le_bytes()); + b.extend([0, 0, 0, b',', 0, 0, 0, 0]); + b.extend(w.to_le_bytes()); + b.extend(h.to_le_bytes()); + b.extend([0, 2, 0, b';']); + b +} + +/// A 54-byte uncompressed 24-bit BMP header declaring `w` × `h` and no pixel data. +fn tiny_bmp(w: i32, h: i32) -> Vec { + let mut b = b"BM".to_vec(); + b.extend(54u32.to_le_bytes()); + b.extend(0u32.to_le_bytes()); + b.extend(54u32.to_le_bytes()); + b.extend(40u32.to_le_bytes()); + b.extend(w.to_le_bytes()); + b.extend(h.to_le_bytes()); + b.extend(1u16.to_le_bytes()); + b.extend(24u16.to_le_bytes()); + b.extend([0u8; 24]); + b +} + +/// Issue #2553: the import probe had no header reader for GIF or BMP and decoded them in full, so +/// a tiny file declaring a canvas just under the 1-gigapixel limit allocated ~4.3 GB (an abort, not +/// an error, when it failed). Both the probe and a decode now refuse a canvas the file's bytes +/// can't fill, before the decoder allocates it. +#[test] +fn tiny_gif_and_bmp_declaring_a_gigapixel_canvas_are_refused_before_allocating() { + for (name, bytes) in [("gif", tiny_gif(32_767, 32_767)), ("bmp", tiny_bmp(32_000, 32_000))] { + assert!(bytes.len() < 64, "{name}: {} bytes", bytes.len()); + let header = read_header(&bytes); + assert!(matches!(&header, Err(Error::Malformed(_, m)) if m.contains("can hold")), "{name}: {header:?}"); + let decoded = decode(&bytes, DecodeOptions::default()); + assert!(matches!(&decoded, Err(Error::Malformed(_, m)) if m.contains("can hold")), "{name}: {:?}", decoded.err()); + } +} + +/// Above 4 MP the probe reads GIF and BMP sizes from the header without decoding; the size agrees +/// with a full decode. +#[test] +fn large_gif_and_bmp_sizes_come_from_the_header() { + let (w, h) = (2_100u32, 2_000u32); + let rgb = vec![90u8; (w * h * 3) as usize]; + let mut bmp = Vec::new(); + image::codecs::bmp::BmpEncoder::new(&mut bmp).encode(&rgb, w, h, image::ExtendedColorType::Rgb8).unwrap(); + let mut gif = Vec::new(); + let frame = image::RgbaImage::from_pixel(w, h, image::Rgba([90, 90, 90, 255])); + image::codecs::gif::GifEncoder::new(&mut gif).encode_frame(image::Frame::new(frame)).unwrap(); + for (name, bytes) in [("bmp", bmp), ("gif", gif)] { + let header = read_header(&bytes).unwrap_or_else(|e| panic!("{name}: {e}")); + assert_eq!((header.width, header.height, header.orientation), (w, h, 1), "{name}"); + let d = decode(&bytes, DecodeOptions::fit(64, 64)).unwrap(); + assert_eq!((d.source_width, d.source_height), (w, h), "{name}"); + } +} + #[test] fn thumbnail_respects_configured_source_pixel_limit() { let image = Rgba8::from_fn(3, 2, |_, _| [40, 80, 120, 255]); diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..cba458a75 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,13 @@ # What's new in LightCraft +## 2026-10-22 + +### A damaged GIF or BMP can no longer crash an import +- A GIF or BMP whose header claims an enormous picture (a file of a few dozen bytes can claim a billion pixels) is + now refused with an error when importing or opening it (issue #2553). Before, LightCraft set aside memory for the + whole picture, several gigabytes, and could close without warning, losing unsaved work. Large GIF and BMP files + are also measured from their header at import instead of being decoded. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges