From c2cdb112b702089ef7784cefa8e5105f961c7528 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 10:55:54 -0700 Subject: [PATCH 1/5] 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 From b55daa91da469fa4cca237a029a54a1984ec92f0 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Thu, 22 Oct 2026 18:31:28 -0700 Subject: [PATCH 2/5] LR-IMP-FORMATS: an RLE BMP that ends early opens; a GIF's bound counts only its LZW data (#2553) Review follow-ups on check_fillable: - RLE8/RLE4 BMPs skip the density check: an end-of-bitmap code zero-fills the rest, so a few bytes are a valid picture of any size (check_size still caps it). Uncompressed BMPs need at least 1 byte per 8 pixels (rows are read in full). - A GIF's bound counts the first frame's LZW sub-block bytes, not comments or palettes, at 2731 pixels per byte (a 4096-pixel code takes 12 bits). - The limits() comment no longer claims it stops the canvas allocation: DynamicImage::from_decoder allocates without consulting max_alloc. - whats-new states the 4 MP cutoff. Co-Authored-By: Claude Opus 5.5 --- crates/codecs/src/other.rs | 78 ++++++++++++++++++++++++++++--------- crates/codecs/tests/fuzz.rs | 47 ++++++++++++++++++++++ docs/whats-new.md | 6 ++- 3 files changed, 111 insertions(+), 20 deletions(-) diff --git a/crates/codecs/src/other.rs b/crates/codecs/src/other.rs index f99bd5f17..43942994f 100644 --- a/crates/codecs/src/other.rs +++ b/crates/codecs/src/other.rs @@ -23,7 +23,7 @@ pub(crate) fn header(bytes: &[u8], format: Format) -> Result<(u32, u32, u16)> { if w == 0 || h == 0 { return Err(Error::Malformed(format, "zero dimension".into())); } - check_fillable(format, bytes.len(), w, h)?; + check_fillable(format, bytes, 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)); @@ -31,31 +31,73 @@ pub(crate) fn header(bytes: &[u8], format: Format) -> Result<(u32, u32, u16)> { 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. +/// The decoder's allocation limit: the RGBA buffer of the largest image `opts` allows. It bounds the +/// decoders' own frame buffers only: `DynamicImage::from_decoder` allocates the canvas without +/// consulting it, so the canvas is bounded by `check_size` and [`check_fillable`] beforehand. 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"))); +/// Refuses a canvas larger than the file's bytes can describe, before the decoder allocates it +/// (issue #2553). Uncompressed BMP rows are stored in full at 1 bit per pixel or more, so the file +/// holds at least one byte per 8 pixels. An RLE BMP has no such bound: an end-of-bitmap code leaves +/// the rest of the picture blank, so it is left to `check_size`. A GIF frame's pixels come from its +/// LZW data alone; a code expands to at most 4096 pixels and takes 12 bits once the table holds +/// that many (fewer bits only while its strings are shorter), so at most 2731 pixels per byte. +fn check_fillable(format: Format, bytes: &[u8], w: u32, h: u32) -> Result<()> { + let (len, per_byte) = match format { + Format::Bmp if bmp_is_rle(bytes) => return Ok(()), + Format::Gif => (gif_image_data_len(bytes).unwrap_or(bytes.len()), 2_731), + _ => (bytes.len(), 8), + }; + if u64::from(w).saturating_mul(u64::from(h)) > (len as u64).saturating_mul(per_byte) { + return Err(Error::Malformed(format, format!("a {w} × {h} canvas is far larger than its {len} bytes of pixel data can hold"))); } Ok(()) } +/// Whether a BMP's pixels are run-length encoded (RLE8 or RLE4, in a header of 40 bytes or more). +fn bmp_is_rle(bytes: &[u8]) -> bool { + let u32_at = |i: usize| bytes.get(i..i + 4).and_then(|b| b.try_into().ok()).map(u32::from_le_bytes); + u32_at(14).is_some_and(|size| size >= 40) && matches!(u32_at(30), Some(1 | 2)) +} + +/// The bytes of LZW data present in a GIF's first frame; its palettes, comments and other +/// extensions describe no pixels. `None` when the blocks before the frame can't be walked. +fn gif_image_data_len(bytes: &[u8]) -> Option { + let table = |flags: u8| if flags & 0x80 != 0 { 3usize << ((flags & 7) + 1) } else { 0 }; + let mut pos = 13 + table(*bytes.get(10)?); + loop { + match *bytes.get(pos)? { + // extension: introducer, label, sub-blocks + 0x21 => pos = sub_blocks(bytes, pos.saturating_add(2)).1, + // image descriptor (10 bytes), local colour table, LZW minimum code size, data sub-blocks + 0x2C => { + let start = pos.saturating_add(10).saturating_add(table(*bytes.get(pos.saturating_add(9))?)).saturating_add(1); + return Some(sub_blocks(bytes, start).0); + } + _ => return None, + } + } +} + +/// The GIF sub-blocks starting at `pos`: the data bytes present, and the position after them. +fn sub_blocks(bytes: &[u8], mut pos: usize) -> (usize, usize) { + let mut data = 0usize; + while let Some(&n) = bytes.get(pos) { + let start = pos.saturating_add(1); + if n == 0 { + return (data, start); + } + let end = start.saturating_add(usize::from(n)); + data = data.saturating_add(end.min(bytes.len()).saturating_sub(start)); + pos = end; + } + (data, pos) +} + 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); @@ -64,7 +106,7 @@ 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)?; + check_fillable(format, bytes, 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) @@ -73,7 +115,7 @@ 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)?; + check_fillable(format, bytes, 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 d52f80ee0..67a5b59c2 100644 --- a/crates/codecs/tests/fuzz.rs +++ b/crates/codecs/tests/fuzz.rs @@ -135,6 +135,53 @@ fn large_gif_and_bmp_sizes_come_from_the_header() { } } +/// An RLE8 BMP may end its pixels with an end-of-bitmap code that leaves the rest blank, so a file +/// of a few dozen bytes is a valid 6 MP picture: it opens, at its declared size. +#[test] +fn an_rle_bmp_that_ends_early_still_opens() { + let (w, h) = (3_000i32, 2_000i32); + let mut b = b"BM".to_vec(); + b.extend(64u32.to_le_bytes()); // file size + b.extend(0u32.to_le_bytes()); + b.extend(62u32.to_le_bytes()); // pixel data offset: 14 + 40 + 2 palette entries + 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(8u16.to_le_bytes()); + b.extend(1u32.to_le_bytes()); // BI_RLE8 + b.extend(2u32.to_le_bytes()); // pixel data size + b.extend([0u8; 8]); + b.extend(2u32.to_le_bytes()); // colours used + b.extend(0u32.to_le_bytes()); + b.extend([0, 0, 0, 0, 255, 255, 255, 0]); + b.extend([0, 1]); // end of bitmap + assert_eq!(b.len(), 64); + let header = read_header(&b).unwrap(); + assert_eq!((header.width, header.height), (w as u32, h as u32)); + let d = decode(&b, DecodeOptions::fit(64, 64)).unwrap(); + assert_eq!((d.source_width, d.source_height), (w as u32, h as u32)); +} + +/// A GIF's comments and other extensions describe no pixels, so padding a tiny GIF with 100 KB of +/// comment doesn't let it claim a gigapixel canvas: only the frame's LZW data counts. +#[test] +fn a_gif_padded_with_comments_still_cannot_claim_a_gigapixel_canvas() { + let mut bytes = tiny_gif(32_767, 32_767); + let mut comment = vec![0x21, 0xFE]; + for _ in 0..400 { + comment.push(255); + comment.extend([b'x'; 255]); + } + comment.push(0); + bytes.splice(13..13, comment); + assert!(bytes.len() > 100_000); + let header = read_header(&bytes); + assert!(matches!(&header, Err(Error::Malformed(_, m)) if m.contains("can hold")), "{header:?}"); + let decoded = decode(&bytes, DecodeOptions::default()); + assert!(matches!(&decoded, Err(Error::Malformed(_, m)) if m.contains("can hold")), "{:?}", decoded.err()); +} + #[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 cba458a75..0f069ff14 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -5,8 +5,10 @@ ### 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. + whole picture, several gigabytes, and could close without warning, losing unsaved work. At import, a GIF or BMP + picture larger than 4 megapixels now has its size read from the header instead of being decoded; smaller ones are + still decoded, so a damaged file is caught then. A run-length-encoded BMP that ends early, leaving the rest of the + picture blank, still opens. ## 2026-10-21 From 1bc7cab6c824e753cbf54b30b4e487276a74ba57 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Fri, 23 Oct 2026 14:37:53 -0700 Subject: [PATCH 3/5] LR-IMP-FORMATS: GIF density from the first frame; mostly blank canvases capped at 4 MP From echelon's review of #2607: - A GIF's LZW data now bounds its first frame's own width x height, not the logical screen's, so a valid GIF with a small first frame on a larger screen imports. - A canvas the file's data leaves mostly blank (a GIF screen larger than its first frame, or an RLE BMP larger than its runs could cover at 255 pixels per 2 bytes) still costs its whole size in from_decoder, so it is limited to 4 MP (SPARSE_MAX_PIXELS) and refused before the decoder allocates it. Densely coded canvases keep the max_pixels limit. Tests: a 64-byte RLE BMP declaring 2001 x 2000 or 32000 x 32000 is refused ("mostly blank"); the RLE BMP that ends early still opens at 2000 x 2000; a GIF with a 1 x 1 first frame opens on a 2000 x 2000 screen and is refused on a 32767 x 32767 one. The first three fail without this change. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PtjHGt5gnCHXgLt4NbGorf --- crates/codecs/src/other.rs | 68 ++++++++++++++++++++++++++-------- crates/codecs/tests/it/fuzz.rs | 64 +++++++++++++++++++++++++++++--- docs/whats-new.md | 5 ++- 3 files changed, 114 insertions(+), 23 deletions(-) diff --git a/crates/codecs/src/other.rs b/crates/codecs/src/other.rs index 43942994f..f537b8ad4 100644 --- a/crates/codecs/src/other.rs +++ b/crates/codecs/src/other.rs @@ -33,27 +33,59 @@ pub(crate) fn header(bytes: &[u8], format: Format) -> Result<(u32, u32, u16)> { /// The decoder's allocation limit: the RGBA buffer of the largest image `opts` allows. It bounds the /// decoders' own frame buffers only: `DynamicImage::from_decoder` allocates the canvas without -/// consulting it, so the canvas is bounded by `check_size` and [`check_fillable`] beforehand. +/// consulting it, so the canvas is bounded by `check_size` and [`check_fillable`] beforehand (a +/// mostly blank one by [`SPARSE_MAX_PIXELS`]). fn limits(opts: &DecodeOptions) -> image::Limits { let mut l = image::Limits::default(); l.max_alloc = Some(opts.max_pixels.saturating_mul(4)); l } +/// The largest canvas allowed that the file's pixel data leaves mostly blank: a GIF screen larger +/// than its first frame, or an RLE BMP whose runs can't cover it. Its decode still allocates the +/// whole canvas (16 MB at this size), so it is held well below `max_pixels`. +const SPARSE_MAX_PIXELS: u64 = 4_000_000; + /// Refuses a canvas larger than the file's bytes can describe, before the decoder allocates it /// (issue #2553). Uncompressed BMP rows are stored in full at 1 bit per pixel or more, so the file -/// holds at least one byte per 8 pixels. An RLE BMP has no such bound: an end-of-bitmap code leaves -/// the rest of the picture blank, so it is left to `check_size`. A GIF frame's pixels come from its -/// LZW data alone; a code expands to at most 4096 pixels and takes 12 bits once the table holds -/// that many (fewer bits only while its strings are shorter), so at most 2731 pixels per byte. +/// holds at least one byte per 8 pixels. A GIF frame's pixels come from its LZW data alone; a code +/// expands to at most 4096 pixels and takes 12 bits once the table holds that many (fewer bits only +/// while its strings are shorter), so at most 2731 pixels per byte. That bounds the first frame's +/// own size; the screen around it may be larger, and is blank there. +/// +/// An RLE BMP has no such bound, since an end-of-bitmap code or a delta leaves the rest blank. Its +/// runs cover at most 255 pixels per 2 bytes, so a canvas beyond 128 pixels per byte is mostly +/// blank. Mostly blank canvases are limited to [`SPARSE_MAX_PIXELS`]. fn check_fillable(format: Format, bytes: &[u8], w: u32, h: u32) -> Result<()> { - let (len, per_byte) = match format { - Format::Bmp if bmp_is_rle(bytes) => return Ok(()), - Format::Gif => (gif_image_data_len(bytes).unwrap_or(bytes.len()), 2_731), - _ => (bytes.len(), 8), + let area = |w: u32, h: u32| u64::from(w).saturating_mul(u64::from(h)); + let unfillable = |what: &str, w: u32, h: u32, len: usize| { + Error::Malformed(format, format!("a {w} × {h} {what} is far larger than its {len} bytes of pixel data can hold")) }; - if u64::from(w).saturating_mul(u64::from(h)) > (len as u64).saturating_mul(per_byte) { - return Err(Error::Malformed(format, format!("a {w} × {h} canvas is far larger than its {len} bytes of pixel data can hold"))); + let canvas = area(w, h); + // the pixels the file's data can cover + let covered = match format { + Format::Gif => { + let (fw, fh, len) = gif_first_frame(bytes).unwrap_or((w, h, bytes.len())); + if area(fw, fh) > (len as u64).saturating_mul(2_731) { + return Err(unfillable("frame", fw, fh, len)); + } + area(fw, fh) + } + Format::Bmp if bmp_is_rle(bytes) => (bytes.len() as u64).saturating_mul(128), + _ => { + if canvas > (bytes.len() as u64).saturating_mul(8) { + return Err(unfillable("canvas", w, h, bytes.len())); + } + canvas + } + }; + if canvas > covered && canvas > SPARSE_MAX_PIXELS { + return Err(Error::Malformed( + format, + format!( + "a {w} × {h} canvas is mostly blank (its pixel data covers at most {covered} pixels), and such pictures are limited to {SPARSE_MAX_PIXELS} pixels" + ), + )); } Ok(()) } @@ -64,19 +96,23 @@ fn bmp_is_rle(bytes: &[u8]) -> bool { u32_at(14).is_some_and(|size| size >= 40) && matches!(u32_at(30), Some(1 | 2)) } -/// The bytes of LZW data present in a GIF's first frame; its palettes, comments and other -/// extensions describe no pixels. `None` when the blocks before the frame can't be walked. -fn gif_image_data_len(bytes: &[u8]) -> Option { +/// A GIF's first frame: its width and height, and the bytes of LZW data present (its palettes, +/// comments and other extensions describe no pixels). `None` when the blocks before the frame +/// can't be walked. +fn gif_first_frame(bytes: &[u8]) -> Option<(u32, u32, usize)> { + let u16_at = |i: usize| bytes.get(i..i.checked_add(2)?).and_then(|b| b.try_into().ok()).map(u16::from_le_bytes); let table = |flags: u8| if flags & 0x80 != 0 { 3usize << ((flags & 7) + 1) } else { 0 }; let mut pos = 13 + table(*bytes.get(10)?); loop { match *bytes.get(pos)? { // extension: introducer, label, sub-blocks 0x21 => pos = sub_blocks(bytes, pos.saturating_add(2)).1, - // image descriptor (10 bytes), local colour table, LZW minimum code size, data sub-blocks + // image descriptor (separator, left, top, width, height, flags), local colour table, + // LZW minimum code size, data sub-blocks 0x2C => { + let (fw, fh) = (u16_at(pos.saturating_add(5))?, u16_at(pos.saturating_add(7))?); let start = pos.saturating_add(10).saturating_add(table(*bytes.get(pos.saturating_add(9))?)).saturating_add(1); - return Some(sub_blocks(bytes, start).0); + return Some((u32::from(fw), u32::from(fh), sub_blocks(bytes, start).0)); } _ => return None, } diff --git a/crates/codecs/tests/it/fuzz.rs b/crates/codecs/tests/it/fuzz.rs index 67a5b59c2..b150f9c78 100644 --- a/crates/codecs/tests/it/fuzz.rs +++ b/crates/codecs/tests/it/fuzz.rs @@ -135,11 +135,8 @@ fn large_gif_and_bmp_sizes_come_from_the_header() { } } -/// An RLE8 BMP may end its pixels with an end-of-bitmap code that leaves the rest blank, so a file -/// of a few dozen bytes is a valid 6 MP picture: it opens, at its declared size. -#[test] -fn an_rle_bmp_that_ends_early_still_opens() { - let (w, h) = (3_000i32, 2_000i32); +/// A 64-byte RLE8 BMP declaring `w` × `h` whose pixels are one end-of-bitmap code: all blank. +fn rle_bmp_ending_at_once(w: i32, h: i32) -> Vec { let mut b = b"BM".to_vec(); b.extend(64u32.to_le_bytes()); // file size b.extend(0u32.to_le_bytes()); @@ -157,12 +154,69 @@ fn an_rle_bmp_that_ends_early_still_opens() { b.extend([0, 0, 0, 0, 255, 255, 255, 0]); b.extend([0, 1]); // end of bitmap assert_eq!(b.len(), 64); + b +} + +/// An RLE8 BMP may end its pixels with an end-of-bitmap code that leaves the rest blank, so a file +/// of a few dozen bytes is a valid picture: up to 4 MP it opens, at its declared size. +#[test] +fn an_rle_bmp_that_ends_early_still_opens() { + let (w, h) = (2_000i32, 2_000i32); + let b = rle_bmp_ending_at_once(w, h); let header = read_header(&b).unwrap(); assert_eq!((header.width, header.height), (w as u32, h as u32)); let d = decode(&b, DecodeOptions::fit(64, 64)).unwrap(); assert_eq!((d.source_width, d.source_height), (w as u32, h as u32)); } +/// A mostly blank canvas still costs its full size to decode, so a tiny RLE BMP can't claim a +/// gigapixel one: canvases the file's runs can't cover are limited to 4 MP, refused before the +/// decoder allocates them. +#[test] +fn a_tiny_rle_bmp_declaring_a_gigapixel_canvas_is_refused_before_allocating() { + for (w, h) in [(2_001, 2_000), (32_000, 32_000)] { + let b = rle_bmp_ending_at_once(w, h); + let header = read_header(&b); + assert!(matches!(&header, Err(Error::Malformed(_, m)) if m.contains("mostly blank")), "{w} × {h}: {header:?}"); + let decoded = decode(&b, DecodeOptions::default()); + assert!(matches!(&decoded, Err(Error::Malformed(_, m)) if m.contains("mostly blank")), "{w} × {h}: {:?}", decoded.err()); + } +} + +/// A `sw` × `sh` GIF screen whose first frame is one pixel (black, from a 2-colour palette). +fn gif_with_one_pixel_frame(sw: u16, sh: u16) -> Vec { + let mut b = b"GIF89a".to_vec(); + b.extend(sw.to_le_bytes()); + b.extend(sh.to_le_bytes()); + b.extend([0x80, 0, 0]); // global colour table of 2 entries + b.extend([0, 0, 0, 255, 255, 255]); + b.extend([b',', 0, 0, 0, 0, 1, 0, 1, 0, 0]); // a 1 × 1 frame at the origin + b.extend([2, 2, 0x44, 0x01, 0]); // LZW: clear, index 0, end + b.push(b';'); + b +} + +/// Density is judged on the first frame's own size, not the screen's: a valid GIF with a small first +/// frame on a larger screen opens, at the screen's size (up to 4 MP, as for other mostly blank +/// canvases). +#[test] +fn a_gif_with_a_small_first_frame_on_a_larger_screen_opens() { + let b = gif_with_one_pixel_frame(2_000, 2_000); + let header = read_header(&b).unwrap(); + assert_eq!((header.width, header.height), (2_000, 2_000)); + let d = decode(&b, DecodeOptions::fit(64, 64)).unwrap(); + assert_eq!((d.source_width, d.source_height), (2_000, 2_000)); +} + +#[test] +fn a_gif_whose_one_pixel_frame_sits_on_a_gigapixel_screen_is_refused_before_allocating() { + let b = gif_with_one_pixel_frame(32_767, 32_767); + let header = read_header(&b); + assert!(matches!(&header, Err(Error::Malformed(_, m)) if m.contains("mostly blank")), "{header:?}"); + let decoded = decode(&b, DecodeOptions::default()); + assert!(matches!(&decoded, Err(Error::Malformed(_, m)) if m.contains("mostly blank")), "{:?}", decoded.err()); +} + /// A GIF's comments and other extensions describe no pixels, so padding a tiny GIF with 100 KB of /// comment doesn't let it claim a gigapixel canvas: only the frame's LZW data counts. #[test] diff --git a/docs/whats-new.md b/docs/whats-new.md index 0f069ff14..db338d155 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -7,8 +7,9 @@ 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. At import, a GIF or BMP picture larger than 4 megapixels now has its size read from the header instead of being decoded; smaller ones are - still decoded, so a damaged file is caught then. A run-length-encoded BMP that ends early, leaving the rest of the - picture blank, still opens. + still decoded, so a damaged file is caught then. A picture that is mostly blank (a run-length-encoded BMP that ends + early, or a GIF whose first frame is smaller than the picture) still opens up to 4 megapixels; a larger one is + refused, because opening it would still set aside memory for the whole picture. ## 2026-10-21 From 24ee45c7f8d4d3acc040241a4dcd4cd33631cec0 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Fri, 23 Oct 2026 16:54:29 -0700 Subject: [PATCH 4/5] LR-IMP-FORMATS: a GIF with no readable first frame is refused From CodeRabbit's review of #2607: when the blocks before the first image descriptor could not be walked, check_fillable fell back to treating the whole file as pixel data and the whole screen as covered, so padding could let a large canvas through the bound. A GIF with no first frame has nothing to decode, so it is now Malformed ("no readable first frame"). The image crate's GIF decoder also stops at such files when it opens them, so this was not reachable through read_header or decode in the cases tried; the bound no longer relies on the two walks agreeing. Unit test a_gif_with_no_readable_first_frame_is_refused calls check_fillable on a GIF whose comment runs to the end of the file; the old fallback returned Ok for its 400 MP screen. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PtjHGt5gnCHXgLt4NbGorf --- crates/codecs/src/other.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/codecs/src/other.rs b/crates/codecs/src/other.rs index f537b8ad4..444e71d39 100644 --- a/crates/codecs/src/other.rs +++ b/crates/codecs/src/other.rs @@ -65,7 +65,10 @@ fn check_fillable(format: Format, bytes: &[u8], w: u32, h: u32) -> Result<()> { // the pixels the file's data can cover let covered = match format { Format::Gif => { - let (fw, fh, len) = gif_first_frame(bytes).unwrap_or((w, h, bytes.len())); + // without a frame there is nothing to decode, and nothing to bound the canvas by + let Some((fw, fh, len)) = gif_first_frame(bytes) else { + return Err(Error::Malformed(format, "no readable first frame".into())); + }; if area(fw, fh) > (len as u64).saturating_mul(2_731) { return Err(unfillable("frame", fw, fh, len)); } @@ -170,3 +173,26 @@ pub(crate) fn decode(bytes: &[u8], format: Format, opts: &DecodeOptions) -> Resu let raw = Raw { width: w as usize, height: h as usize, model, alpha, premultiplied: false, buf: Buf::U8(buf), bit_depth: 8 }; finish(format, raw, Meta { icc, ..Default::default() }, (w, h), opts) } + +#[cfg(test)] +mod tests { + use super::*; + + /// A GIF whose blocks can't be walked to a first frame (here a comment that runs to the end of the + /// file) is refused, instead of the whole file being counted as pixel data for its 400 MP screen. + /// The `image` decoder also stops at such a file when it opens it, but the canvas bound doesn't + /// rely on the two walks agreeing. + #[test] + fn a_gif_with_no_readable_first_frame_is_refused() { + let mut b = b"GIF89a".to_vec(); + b.extend(20_000u16.to_le_bytes()); + b.extend(20_000u16.to_le_bytes()); + b.extend([0, 0, 0, 0x21, 0xFE]); + for _ in 0..600 { + b.push(255); + b.extend([b'x'; 255]); + } + let r = check_fillable(Format::Gif, &b, 20_000, 20_000); + assert!(matches!(&r, Err(Error::Malformed(_, m)) if m.contains("no readable first frame")), "{r:?}"); + } +} From fbbf5c0c53c8a07b3c7ecf26f3b2a324d1a89029 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Sat, 24 Oct 2026 07:46:31 -0700 Subject: [PATCH 5/5] LR-IMP-FORMATS: GIF and BMP coverage comes from the pixel data, not the file's length (#2553) Padding counted as pixel data: a file padded with junk could still claim a canvas up to max_pixels, which the decoder allocates before it finds the data missing. - Uncompressed BMP: the rows the header describes (bits per pixel, 4-byte row padding) must all be present after the pixel-data offset; the decoder reads every row in full. - RLE BMP: the commands are walked as the decoder paints them (runs clipped at the row's end, end-of-row, delta, end-of-bitmap); bytes after the end draw nothing. - GIF: the first frame's LZW codes are walked up to the end code, keeping only the length of each table string; data sub-blocks after the end code draw nothing. A canvas the data leaves mostly blank stays limited to 4 MP. New tests: one padded file per case (all four were accepted before), and the LZW walk against frames the encoder wrote. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016TuX1qw9ieyuJ1ZFrrUxzc --- crates/codecs/src/other.rs | 243 ++++++++++++++++++++++++++++----- crates/codecs/tests/it/fuzz.rs | 44 ++++++ 2 files changed, 254 insertions(+), 33 deletions(-) diff --git a/crates/codecs/src/other.rs b/crates/codecs/src/other.rs index 755e83a71..021cb08c8 100644 --- a/crates/codecs/src/other.rs +++ b/crates/codecs/src/other.rs @@ -41,46 +41,57 @@ fn limits(opts: &DecodeOptions) -> image::Limits { l } -/// The largest canvas allowed that the file's pixel data leaves mostly blank: a GIF screen larger -/// than its first frame, or an RLE BMP whose runs can't cover it. Its decode still allocates the +/// The largest canvas allowed that the file's pixel data leaves mostly blank: a GIF screen its +/// first frame's codes don't cover, or an RLE BMP whose runs don't. Its decode still allocates the /// whole canvas (16 MB at this size), so it is held well below `max_pixels`. const SPARSE_MAX_PIXELS: u64 = 4_000_000; -/// Refuses a canvas larger than the file's bytes can describe, before the decoder allocates it -/// (issue #2553). Uncompressed BMP rows are stored in full at 1 bit per pixel or more, so the file -/// holds at least one byte per 8 pixels. A GIF frame's pixels come from its LZW data alone; a code -/// expands to at most 4096 pixels and takes 12 bits once the table holds that many (fewer bits only -/// while its strings are shorter), so at most 2731 pixels per byte. That bounds the first frame's -/// own size; the screen around it may be larger, and is blank there. +/// Refuses a canvas larger than the file's pixel data can describe, before the decoder allocates it +/// (issue #2553). Coverage is measured from the pixel data itself, never from the file's length, so +/// padding (comments, bytes after an end code, trailing bytes) covers nothing: /// -/// An RLE BMP has no such bound, since an end-of-bitmap code or a delta leaves the rest blank. Its -/// runs cover at most 255 pixels per 2 bytes, so a canvas beyond 128 pixels per byte is mostly -/// blank. Mostly blank canvases are limited to [`SPARSE_MAX_PIXELS`]. +/// - Uncompressed BMP rows are read in full, padding included, from the offset the file header +/// gives, so the data must hold every row: anything shorter fails to decode once the canvas is +/// allocated, and is refused here instead. +/// - An RLE BMP covers the pixels its runs and absolute blocks draw, as the decoder paints them; an +/// end-of-row, end-of-bitmap or delta code leaves the rest blank. +/// - A GIF covers the pixels its first frame's LZW codes draw up to the end code. A code expands to +/// at most 4096 pixels and takes 12 bits once the table holds that many (fewer bits only while its +/// strings are shorter), so a frame larger than 2731 pixels per byte of LZW data is refused +/// outright. The screen around the frame may be larger, and is blank there. +/// +/// Canvases the data leaves mostly blank are limited to [`SPARSE_MAX_PIXELS`]. fn check_fillable(format: Format, bytes: &[u8], w: u32, h: u32) -> Result<()> { let area = |w: u32, h: u32| u64::from(w).saturating_mul(u64::from(h)); - let unfillable = |what: &str, w: u32, h: u32, len: usize| { - Error::Malformed(format, format!("a {w} × {h} {what} is far larger than its {len} bytes of pixel data can hold")) - }; let canvas = area(w, h); // the pixels the file's data can cover let covered = match format { Format::Gif => { // without a frame there is nothing to decode, and nothing to bound the canvas by - let Some((fw, fh, len)) = gif_first_frame(bytes) else { + let Some(frame) = gif_first_frame(bytes) else { return Err(Error::Malformed(format, "no readable first frame".into())); }; + let (fw, fh, len) = (frame.width, frame.height, frame.data_len); if area(fw, fh) > (len as u64).saturating_mul(2_731) { - return Err(unfillable("frame", fw, fh, len)); + return Err(Error::Malformed(format, format!("a {fw} × {fh} frame is far larger than its {len} bytes of pixel data can hold"))); } - area(fw, fh) + gif_frame_pixels(bytes, frame.codes, area(fw, fh)) } - Format::Bmp if bmp_is_rle(bytes) => (bytes.len() as u64).saturating_mul(128), - _ => { - if canvas > (bytes.len() as u64).saturating_mul(8) { - return Err(unfillable("canvas", w, h, bytes.len())); + _ => match bmp_layout(bytes) { + BmpLayout::Rle { offset, rle4 } => bmp_rle_pixels(bytes, offset, rle4, w, h), + BmpLayout::Rows { offset, bits } => { + let row = u64::from(w).saturating_mul(u64::from(bits)).div_ceil(32).saturating_mul(4); + let needed = row.saturating_mul(u64::from(h)); + let held = bytes.len().saturating_sub(offset) as u64; + if held < needed { + return Err(Error::Malformed( + format, + format!("a {w} × {h} canvas is larger than its {held} bytes of pixel data can hold ({needed} needed)"), + )); + } + canvas } - canvas - } + }, }; if canvas > covered && canvas > SPARSE_MAX_PIXELS { return Err(Error::Malformed( @@ -93,16 +104,91 @@ fn check_fillable(format: Format, bytes: &[u8], w: u32, h: u32) -> Result<()> { Ok(()) } -/// Whether a BMP's pixels are run-length encoded (RLE8 or RLE4, in a header of 40 bytes or more). -fn bmp_is_rle(bytes: &[u8]) -> bool { - let u32_at = |i: usize| bytes.get(i..i + 4).and_then(|b| b.try_into().ok()).map(u32::from_le_bytes); - u32_at(14).is_some_and(|size| size >= 40) && matches!(u32_at(30), Some(1 | 2)) +/// Where a BMP's pixels start (the offset in its file header, which the decoder seeks to) and how +/// they are stored. +enum BmpLayout { + /// RLE8 or RLE4 commands. + Rle { offset: usize, rle4: bool }, + /// Rows of `bits` per pixel, each padded to 4 bytes. + Rows { offset: usize, bits: u16 }, } -/// A GIF's first frame: its width and height, and the bytes of LZW data present (its palettes, -/// comments and other extensions describe no pixels). `None` when the blocks before the frame -/// can't be walked. -fn gif_first_frame(bytes: &[u8]) -> Option<(u32, u32, usize)> { +/// The layout the decoder reads, from the file header and the bitmap header: a 12-byte core header +/// (bits per pixel at 24, never compressed) or a 40-byte or larger one (bits at 28, compression at +/// 30). Fields the decoder would have refused are read as 1 bit per pixel at offset 0, the most +/// lenient layout. +fn bmp_layout(bytes: &[u8]) -> BmpLayout { + let u16_at = |i: usize| bytes.get(i..i.checked_add(2)?).and_then(|b| b.try_into().ok()).map(u16::from_le_bytes); + let u32_at = |i: usize| bytes.get(i..i.checked_add(4)?).and_then(|b| b.try_into().ok()).map(u32::from_le_bytes); + let offset = u32_at(10).and_then(|o| usize::try_from(o).ok()).unwrap_or(0); + let (bits, compression) = match u32_at(14) { + Some(12) => (u16_at(24), Some(0)), + _ => (u16_at(28), u32_at(30)), + }; + match compression { + Some(1) => BmpLayout::Rle { offset, rle4: false }, + Some(2) => BmpLayout::Rle { offset, rle4: true }, + _ => BmpLayout::Rows { offset, bits: bits.filter(|&b| b > 0).unwrap_or(1) }, + } +} + +/// The pixels an RLE8 or RLE4 BMP's commands draw on a `w` × `h` canvas, read the way the decoder +/// paints them: a run is clipped at the end of its row, an end-of-row code moves to the next row, a +/// delta skips pixels (left blank), and an end-of-bitmap code, a truncated command or the last row +/// ends the picture. Every command takes at least 2 bytes, so the walk is linear in the file. +fn bmp_rle_pixels(bytes: &[u8], offset: usize, rle4: bool, w: u32, h: u32) -> u64 { + let (w, h) = (u64::from(w), u64::from(h)); + let (mut pos, mut x, mut y, mut covered) = (offset, 0u64, 0u64, 0u64); + while y < h { + let (Some(&count), Some(&value)) = (bytes.get(pos), bytes.get(pos.saturating_add(1))) else { + break; + }; + pos = pos.saturating_add(2); + match (count, value) { + // end of row + (0, 0) => (x, y) = (0, y.saturating_add(1)), + // end of bitmap + (0, 1) => break, + // delta: right and down + (0, 2) => { + let (Some(&dx), Some(&dy)) = (bytes.get(pos), bytes.get(pos.saturating_add(1))) else { + break; + }; + pos = pos.saturating_add(2); + (x, y) = (x.saturating_add(u64::from(dx)), y.saturating_add(u64::from(dy))); + } + // absolute: `value` pixels stored one per byte (RLE8) or nibble (RLE4), padded to 2 bytes + (0, n) => { + let len = if rle4 { usize::from(n).div_ceil(2) } else { usize::from(n) }; + pos = pos.saturating_add(len + (len & 1)); + if pos > bytes.len() { + break; + } + covered = covered.saturating_add(u64::from(n).min(w.saturating_sub(x))); + x = x.saturating_add(u64::from(n)); + } + // a run of `count` pixels + (n, _) => { + covered = covered.saturating_add(u64::from(n).min(w.saturating_sub(x))); + x = x.saturating_add(u64::from(n)); + } + } + } + covered +} + +/// A GIF's first frame, from its image descriptor. +struct GifFrame { + width: u32, + height: u32, + /// Bytes of LZW data present (its palettes, comments and other extensions describe no pixels). + data_len: usize, + /// Position of its LZW minimum code size, which its data sub-blocks follow. + codes: usize, +} + +/// A GIF's first frame. `None` when the blocks before the frame can't be walked. +fn gif_first_frame(bytes: &[u8]) -> Option { let u16_at = |i: usize| bytes.get(i..i.checked_add(2)?).and_then(|b| b.try_into().ok()).map(u16::from_le_bytes); let table = |flags: u8| if flags & 0x80 != 0 { 3usize << ((flags & 7) + 1) } else { 0 }; let mut pos = 13 + table(*bytes.get(10)?); @@ -114,14 +200,80 @@ fn gif_first_frame(bytes: &[u8]) -> Option<(u32, u32, usize)> { // LZW minimum code size, data sub-blocks 0x2C => { let (fw, fh) = (u16_at(pos.saturating_add(5))?, u16_at(pos.saturating_add(7))?); - let start = pos.saturating_add(10).saturating_add(table(*bytes.get(pos.saturating_add(9))?)).saturating_add(1); - return Some((u32::from(fw), u32::from(fh), sub_blocks(bytes, start).0)); + let codes = pos.saturating_add(10).saturating_add(table(*bytes.get(pos.saturating_add(9))?)); + let data_len = sub_blocks(bytes, codes.saturating_add(1)).0; + return Some(GifFrame { width: u32::from(fw), height: u32::from(fh), data_len, codes }); } _ => return None, } } } +/// The pixels a GIF frame's LZW codes draw, at most `max` (the frame's size): the codes are walked +/// up to the end code, an invalid code or the end of the data, keeping only the length of each +/// table string, never the pixels. Data after the end code draws nothing. Codes are packed +/// least-significant bit first across the data sub-blocks and start one bit wider than the minimum +/// code size; a code is one bit wider each time the table fills its width, up to 12 bits, and a +/// clear code starts the table again (GIF89a, appendix F). +fn gif_frame_pixels(bytes: &[u8], codes: usize, max: u64) -> u64 { + // the `gif` decoder accepts minimum code sizes of 1 to 11 + let Some(min_size) = bytes.get(codes).copied().filter(|s| (1..=11).contains(s)) else { + return 0; + }; + let clear = 1usize << min_size; + let end = clear + 1; + let mut lens = [1u16; 4096]; // string length of each code; literals (below `clear`) are 1 + let (mut width, mut next, mut prev) = (u32::from(min_size) + 1, end + 1, None::); + let (mut bits, mut held, mut covered) = (0u32, 0u32, 0u64); + let mut pos = codes.saturating_add(1); + while let Some(&n) = bytes.get(pos) { + let start = pos.saturating_add(1); + pos = start.saturating_add(usize::from(n)); + if n == 0 { + break; + } + for &byte in bytes.get(start..pos.min(bytes.len())).unwrap_or_default() { + bits |= u32::from(byte) << held; + held += 8; + while held >= width { + let code = (bits & ((1 << width) - 1)) as usize; + bits >>= width; + held -= width; + if code == clear { + (width, next, prev) = (u32::from(min_size) + 1, end + 1, None); + continue; + } + let len_of = |c: usize| lens.get(c).copied().unwrap_or(1); + let len = match prev { + _ if code == end => return covered, + None if code < clear => 1, + Some(_) if code < next => len_of(code), + // the string just being defined: the previous one and its first pixel + Some(p) if code == next => len_of(p).saturating_add(1), + _ => return covered, + }; + // each code after the first defines the next string: the previous one and a pixel + let defined = prev.map(|p| len_of(p).saturating_add(1)); + if let Some(defined) = defined + && let Some(slot) = lens.get_mut(next) + { + *slot = defined; + next += 1; + if next == 1 << width && width < 12 { + width += 1; + } + } + prev = Some(code); + covered = covered.saturating_add(u64::from(len)); + if covered >= max { + return max; + } + } + } + } + covered +} + /// The GIF sub-blocks starting at `pos`: the data bytes present, and the position after them. fn sub_blocks(bytes: &[u8], mut pos: usize) -> (usize, usize) { let mut data = 0usize; @@ -273,4 +425,29 @@ mod tests { let r = check_fillable(Format::Gif, &b, 20_000, 20_000); assert!(matches!(&r, Err(Error::Malformed(_, m)) if m.contains("no readable first frame")), "{r:?}"); } + + /// The LZW walk counts exactly the pixels an encoder wrote up to its end code: a frame of one + /// colour (long strings, the smallest codes), a 256-colour gradient and noise (a table that fills + /// up and is cleared again). + #[test] + fn the_lzw_walk_counts_every_pixel_an_encoder_wrote() { + let (w, h) = (640u32, 480u32); + let mut seed = 2_553u32; + let mut noise = move || { + seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); + (seed >> 24) as u8 + }; + let frames = [ + ("solid", image::RgbaImage::from_pixel(w, h, image::Rgba([90, 90, 90, 255]))), + ("gradient", image::RgbaImage::from_fn(w, h, |x, y| image::Rgba([(x / 40 * 16) as u8, (y / 30 * 16) as u8, 0, 255]))), + ("noise", image::RgbaImage::from_fn(w, h, |_, _| image::Rgba([noise(), noise(), noise(), 255]))), + ]; + for (name, frame) in frames { + let mut gif = Vec::new(); + image::codecs::gif::GifEncoder::new_with_speed(&mut gif, 30).encode_frame(image::Frame::new(frame)).unwrap(); + let first = gif_first_frame(&gif).unwrap(); + assert_eq!((first.width, first.height), (w, h), "{name}"); + assert_eq!(gif_frame_pixels(&gif, first.codes, u64::MAX), u64::from(w * h), "{name}"); + } + } } diff --git a/crates/codecs/tests/it/fuzz.rs b/crates/codecs/tests/it/fuzz.rs index b150f9c78..5139d71ba 100644 --- a/crates/codecs/tests/it/fuzz.rs +++ b/crates/codecs/tests/it/fuzz.rs @@ -236,6 +236,50 @@ fn a_gif_padded_with_comments_still_cannot_claim_a_gigapixel_canvas() { assert!(matches!(&decoded, Err(Error::Malformed(_, m)) if m.contains("can hold")), "{:?}", decoded.err()); } +/// Padding is not pixel data: coverage comes from what the pixel data draws, not from the file's +/// length. Each file here is padded past what the old length-based bounds (8 pixels per byte for +/// uncompressed BMP, 128 for RLE, 2731 per LZW byte for a GIF frame) needed to accept its 9 MP +/// canvas, and each draws far less, so each is refused before the decoder allocates the canvas. +#[test] +fn files_padded_with_junk_cannot_claim_a_canvas_their_pixel_data_doesnt_draw() { + let refused = |name: &str, bytes: &[u8], why: &str| { + let header = read_header(bytes); + assert!(matches!(&header, Err(Error::Malformed(_, m)) if m.contains(why)), "{name}: {header:?}"); + let decoded = decode(bytes, DecodeOptions::default()); + assert!(matches!(&decoded, Err(Error::Malformed(_, m)) if m.contains(why)), "{name}: {:?}", decoded.err()); + }; + // a 24-bit BMP needs 3 bytes a pixel (27 MB here); 1.2 MB of zeros used to pass as 8 pixels a byte + let mut bmp = tiny_bmp(3_000, 3_000); + bmp.resize(bmp.len() + 1_200_000, 0); + refused("uncompressed bmp", &bmp, "can hold"); + // an RLE BMP's picture ends at its end-of-bitmap code; 100 KB of runs after it draw nothing + let mut rle = rle_bmp_ending_at_once(3_000, 3_000); + rle.extend([255, 1].repeat(50_000)); + refused("rle bmp padded after its end", &rle, "mostly blank"); + // runs that never end their row are clipped to it: 100 KB of them draw one 3000-pixel row + let mut rle = rle_bmp_ending_at_once(3_000, 3_000); + rle.truncate(rle.len() - 2); + rle.extend([255, 1].repeat(50_000)); + refused("rle bmp padded within one row", &rle, "mostly blank"); + // a GIF frame's picture ends at its LZW end code; 4 KB of data sub-blocks after it draw nothing + let mut gif = b"GIF89a".to_vec(); + gif.extend(3_000u16.to_le_bytes()); + gif.extend(3_000u16.to_le_bytes()); + gif.extend([0x80, 0, 0, 0, 0, 0, 255, 255, 255]); // 2-colour global table + gif.extend([b',', 0, 0, 0, 0]); + gif.extend(3_000u16.to_le_bytes()); + gif.extend(3_000u16.to_le_bytes()); + gif.extend([0, 2]); // no local table; LZW minimum code size 2 + gif.extend([255, 0x2C]); // a full sub-block: 3-bit clear (4) then end (5), then padding + gif.extend([0xFF; 254]); + for _ in 0..15 { + gif.push(255); + gif.extend([0xFF; 255]); + } + gif.extend([0, b';']); + refused("gif padded after its end code", &gif, "mostly blank"); +} + #[test] fn thumbnail_respects_configured_source_pixel_limit() { let image = Rgba8::from_fn(3, 2, |_, _| [40, 80, 120, 255]);