From 1bc7cab6c824e753cbf54b30b4e487276a74ba57 Mon Sep 17 00:00:00 2001 From: insideTheFlux Date: Fri, 23 Oct 2026 14:37:53 -0700 Subject: [PATCH] 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