From 70451cf4dbe0614f3325a9d0c68ba8b3ca71faab Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 13 Nov 2026 11:31:05 +0000 Subject: [PATCH 1/5] feat: cap individual gif and webp frame decode allocations --- crates/gif/src/frames.rs | 13 +++++++++++-- crates/webp/src/frames.rs | 16 ++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/gif/src/frames.rs b/crates/gif/src/frames.rs index fdaec81b7..3b511c404 100644 --- a/crates/gif/src/frames.rs +++ b/crates/gif/src/frames.rs @@ -22,13 +22,17 @@ pub struct FrameLimits { pub max_bytes: u64, /// Largest canvas (logical screen) in bytes at 4 bytes a pixel. Default 512 MiB. pub max_canvas_bytes: u64, + /// Largest single decoder allocation. `None` uses twice `max_canvas_bytes`, matching the + /// decoder's working-buffer allowance. Set this when an application has a tighter allocator + /// budget. Default `None`. + pub max_decode_bytes: Option, /// Merge a frame identical to the one before into it (its delay added). Default off. pub merge_identical: bool, } impl Default for FrameLimits { fn default() -> Self { - FrameLimits { max_frames: 100_000, max_bytes: 1 << 30, max_canvas_bytes: 512 << 20, merge_identical: false } + FrameLimits { max_frames: 100_000, max_bytes: 1 << 30, max_canvas_bytes: 512 << 20, max_decode_bytes: None, merge_identical: false } } } @@ -93,6 +97,10 @@ pub fn read_frames(bytes: &[u8], limits: &FrameLimits) -> Result limits.max_canvas_bytes { return Err(FrameError::LimitExceeded(format!("a {width}x{height} canvas is over {} bytes", limits.max_canvas_bytes))); } + let max_decode_bytes = limits.max_decode_bytes.unwrap_or_else(|| limits.max_canvas_bytes.saturating_mul(2)); + if canvas > max_decode_bytes { + return Err(FrameError::LimitExceeded(format!("a {width}x{height} canvas is over the {max_decode_bytes} byte decode-allocation limit"))); + } // The decoder sets up the whole logical screen before the first frame: the file must back it. if !crate::plausible(info.bytes_to_back(4, 1 << 28), bytes.len(), 1 << 14) { return Err(FrameError::LimitExceeded(format!("a {} byte file cannot hold a {width}x{height} picture", bytes.len()))); @@ -100,7 +108,7 @@ pub fn read_frames(bytes: &[u8], limits: &FrameLimits) -> Result, /// Merge a frame identical to the one before into it (its delay added). Default off. pub merge_identical: bool, } impl Default for FrameLimits { fn default() -> Self { - FrameLimits { max_frames: 100_000, max_bytes: 1 << 30, max_canvas_bytes: 512 << 20, merge_identical: false } + FrameLimits { max_frames: 100_000, max_bytes: 1 << 30, max_canvas_bytes: 512 << 20, max_decode_bytes: None, merge_identical: false } } } @@ -97,10 +101,17 @@ pub fn read_frames(bytes: &[u8], limits: &FrameLimits) -> Result limits.max_canvas_bytes { return Err(FrameError::LimitExceeded(format!("a {}x{} canvas is over {} bytes", info.width, info.height, limits.max_canvas_bytes))); } + let max_decode_bytes = limits.max_decode_bytes.unwrap_or_else(|| limits.max_canvas_bytes.saturating_mul(2)); + if canvas > max_decode_bytes { + return Err(FrameError::LimitExceeded(format!( + "a {}x{} canvas is over the {max_decode_bytes} byte decode-allocation limit", + info.width, info.height + ))); + } let malformed = |e: image::ImageError| FrameError::Malformed(e.to_string()); let mut decoder = WebPDecoder::new(Cursor::new(bytes)).map_err(malformed)?; let mut il = image::Limits::default(); - il.max_alloc = Some(limits.max_canvas_bytes.saturating_mul(2)); + il.max_alloc = Some(max_decode_bytes); decoder.set_limits(il).map_err(malformed)?; let (width, height) = decoder.dimensions(); if !decoder.has_animation() { @@ -243,6 +254,7 @@ mod tests { assert_eq!((a.frames.len(), a.stop), (2, Stop::ByteLimit)); assert!(matches!(read_frames(&bytes, &FrameLimits { max_canvas_bytes: 23, ..FrameLimits::default() }), Err(FrameError::LimitExceeded(_)))); assert!(read_frames(&bytes, &FrameLimits { max_canvas_bytes: 24, ..FrameLimits::default() }).is_ok()); + assert!(read_frames(&bytes, &FrameLimits { max_decode_bytes: Some(1), ..FrameLimits::default() }).is_err()); } #[test] From 9496c938812c8f695ac0671f16d728b51b260940 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 13 Nov 2026 18:25:07 +0000 Subject: [PATCH 2/5] fix: classify gif limits and clarify webp canvas cap --- crates/gif/Cargo.toml | 1 + crates/gif/src/frames.rs | 53 ++++++++++++++++++++++++++++++++++++++- crates/webp/src/frames.rs | 21 +++------------- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/crates/gif/Cargo.toml b/crates/gif/Cargo.toml index ac2b599d5..62abd418e 100644 --- a/crates/gif/Cargo.toml +++ b/crates/gif/Cargo.toml @@ -19,4 +19,5 @@ image = { version = "0.25", default-features = false, features = ["gif"], option [dev-dependencies] # Writes the animated test files the scan is checked against; not a product dependency. +gif = "0.14" image = { version = "0.25", default-features = false, features = ["gif"] } diff --git a/crates/gif/src/frames.rs b/crates/gif/src/frames.rs index 3b511c404..66a731c98 100644 --- a/crates/gif/src/frames.rs +++ b/crates/gif/src/frames.rs @@ -122,7 +122,14 @@ fn collect(width: u32, height: u32, frames: image::Frames<'_>, limits: &FrameLim for frame in frames { let frame = match frame { Ok(f) => f, - Err(e) if out.is_empty() => return Err(FrameError::Malformed(e.to_string())), + Err(e) if out.is_empty() => { + let message = e.to_string(); + return Err(if matches!(e, image::ImageError::Limits(_)) { + FrameError::LimitExceeded(message) + } else { + FrameError::Malformed(message) + }); + } Err(e) => { stop = Stop::Damaged(e.to_string()); break; @@ -158,6 +165,7 @@ fn collect(width: u32, height: u32, frames: image::Frames<'_>, limits: &FrameLim mod tests { #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] use super::*; + use std::borrow::Cow; /// An animated GIF of solid 3 × 2 frames, written by `image`: (red level, delay in hundredths). fn gif(frames: &[(u8, u32)]) -> Vec { @@ -184,6 +192,39 @@ mod tests { assert!(a.frames.iter().all(|f| f.rgba.len() == 24)); } + #[test] + fn partial_frames_composite_and_background_disposal_is_applied() { + let palette = [0, 0, 0, 255, 0, 0, 0, 0, 255, 0, 255, 0]; + let (mut bytes, first, second, third) = (Vec::new(), [1], [2], [0]); + { + let mut encoder = gif::Encoder::new(&mut bytes, 2, 1, &palette).unwrap(); + let a = gif::Frame { width: 1, height: 1, buffer: Cow::Borrowed(&first), transparent: Some(0), delay: 2, ..gif::Frame::default() }; + encoder.write_frame(&a).unwrap(); + + let b = gif::Frame { + left: 1, + width: 1, + height: 1, + buffer: Cow::Borrowed(&second), + transparent: Some(0), + dispose: gif::DisposalMethod::Background, + delay: 3, + ..gif::Frame::default() + }; + encoder.write_frame(&b).unwrap(); + + let c = gif::Frame { width: 1, height: 1, buffer: Cow::Borrowed(&third), transparent: Some(0), delay: 4, ..gif::Frame::default() }; + encoder.write_frame(&c).unwrap(); + } + + let a = read_frames(&bytes, &FrameLimits::default()).unwrap(); + assert_eq!((a.width, a.height, a.stop), (2, 1, Stop::End)); + assert_eq!(a.frames.iter().map(|f| f.delay_ms).collect::>(), [20, 30, 40]); + assert_eq!(a.frames[0].rgba, [255, 0, 0, 255, 0, 0, 0, 0]); + assert_eq!(a.frames[1].rgba, [255, 0, 0, 255, 0, 0, 255, 255]); + assert_eq!(a.frames[2].rgba, [255, 0, 0, 255, 0, 0, 0, 0]); + } + #[test] fn identical_frames_merge_when_asked() { let bytes = gif(&[(10, 5), (10, 7), (200, 1), (200, 1), (10, 2)]); @@ -206,6 +247,16 @@ mod tests { assert!(read_frames(&bytes, &FrameLimits { max_decode_bytes: Some(1), ..FrameLimits::default() }).is_err()); } + #[test] + fn decoder_allocation_limit_is_not_reported_as_malformed_input() { + let mut bytes = gif(&[(1, 1)]); + // The logical screen is 1×1, while the frame data still decodes to 3×2. The canvas + // passes the eight-byte decode cap, but the decoder's frame allocation does not. + bytes[6..10].copy_from_slice(&[1, 0, 1, 0]); + let limits = FrameLimits { max_canvas_bytes: 4, max_decode_bytes: Some(8), ..FrameLimits::default() }; + assert!(matches!(read_frames(&bytes, &limits), Err(FrameError::LimitExceeded(_)))); + } + #[test] fn damage_after_the_first_frame_keeps_what_came_before() { let bytes = gif(&[(1, 1), (2, 1), (3, 1)]); diff --git a/crates/webp/src/frames.rs b/crates/webp/src/frames.rs index 68e5d72e9..1a367eb1b 100644 --- a/crates/webp/src/frames.rs +++ b/crates/webp/src/frames.rs @@ -24,19 +24,17 @@ pub struct FrameLimits { pub max_frames: usize, /// Most bytes of kept pictures (4 bytes a pixel each). Default 1 GiB. pub max_bytes: u64, - /// Largest canvas (logical screen) in bytes at 4 bytes a pixel. Default 512 MiB. + /// Largest canvas (logical screen) in bytes at 4 bytes a pixel. Default 512 MiB. This is a + /// pre-decode canvas check; image 0.25.10's WebP decoder does not enforce its working-buffer + /// allocations through the `ImageDecoder::set_limits` hook. pub max_canvas_bytes: u64, - /// Largest single decoder allocation. `None` uses twice `max_canvas_bytes`, matching the - /// decoder's working-buffer allowance. Set this when an application has a tighter allocator - /// budget. Default `None`. - pub max_decode_bytes: Option, /// Merge a frame identical to the one before into it (its delay added). Default off. pub merge_identical: bool, } impl Default for FrameLimits { fn default() -> Self { - FrameLimits { max_frames: 100_000, max_bytes: 1 << 30, max_canvas_bytes: 512 << 20, max_decode_bytes: None, merge_identical: false } + FrameLimits { max_frames: 100_000, max_bytes: 1 << 30, max_canvas_bytes: 512 << 20, merge_identical: false } } } @@ -101,18 +99,8 @@ pub fn read_frames(bytes: &[u8], limits: &FrameLimits) -> Result limits.max_canvas_bytes { return Err(FrameError::LimitExceeded(format!("a {}x{} canvas is over {} bytes", info.width, info.height, limits.max_canvas_bytes))); } - let max_decode_bytes = limits.max_decode_bytes.unwrap_or_else(|| limits.max_canvas_bytes.saturating_mul(2)); - if canvas > max_decode_bytes { - return Err(FrameError::LimitExceeded(format!( - "a {}x{} canvas is over the {max_decode_bytes} byte decode-allocation limit", - info.width, info.height - ))); - } let malformed = |e: image::ImageError| FrameError::Malformed(e.to_string()); let mut decoder = WebPDecoder::new(Cursor::new(bytes)).map_err(malformed)?; - let mut il = image::Limits::default(); - il.max_alloc = Some(max_decode_bytes); - decoder.set_limits(il).map_err(malformed)?; let (width, height) = decoder.dimensions(); if !decoder.has_animation() { let mut rgba = vec![0u8; usize::try_from(canvas).map_err(|_| FrameError::LimitExceeded("canvas".into()))?]; @@ -254,7 +242,6 @@ mod tests { assert_eq!((a.frames.len(), a.stop), (2, Stop::ByteLimit)); assert!(matches!(read_frames(&bytes, &FrameLimits { max_canvas_bytes: 23, ..FrameLimits::default() }), Err(FrameError::LimitExceeded(_)))); assert!(read_frames(&bytes, &FrameLimits { max_canvas_bytes: 24, ..FrameLimits::default() }).is_ok()); - assert!(read_frames(&bytes, &FrameLimits { max_decode_bytes: Some(1), ..FrameLimits::default() }).is_err()); } #[test] From 1128493ccc4dbe1742d6daf6f34e20a0e9182815 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 13 Nov 2026 18:32:23 +0000 Subject: [PATCH 3/5] chore: update lockfile for GIF regression fixture --- Cargo.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.lock b/Cargo.lock index 6511d9021..f2fbe2d03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1622,6 +1622,7 @@ dependencies = [ name = "craft-gif" version = "0.1.0" dependencies = [ + "gif", "image", ] From 9b85ed000f77a999e4c3b5e27d47594a26fc6c48 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 13 Nov 2026 18:44:33 +0000 Subject: [PATCH 4/5] Fix WebP shared decoder clippy warning --- crates/webp/src/frames.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/webp/src/frames.rs b/crates/webp/src/frames.rs index 1a367eb1b..e97118482 100644 --- a/crates/webp/src/frames.rs +++ b/crates/webp/src/frames.rs @@ -100,7 +100,7 @@ pub fn read_frames(bytes: &[u8], limits: &FrameLimits) -> Result Date: Fri, 13 Nov 2026 19:43:38 +0000 Subject: [PATCH 5/5] Clarify GIF decoder combined allocation budget --- crates/gif/src/frames.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/gif/src/frames.rs b/crates/gif/src/frames.rs index 66a731c98..da65b483e 100644 --- a/crates/gif/src/frames.rs +++ b/crates/gif/src/frames.rs @@ -22,9 +22,9 @@ pub struct FrameLimits { pub max_bytes: u64, /// Largest canvas (logical screen) in bytes at 4 bytes a pixel. Default 512 MiB. pub max_canvas_bytes: u64, - /// Largest single decoder allocation. `None` uses twice `max_canvas_bytes`, matching the - /// decoder's working-buffer allowance. Set this when an application has a tighter allocator - /// budget. Default `None`. + /// Combined decoder working-allocation budget. The decoder reserves its persistent canvas + /// before the frame buffer, so both count against this limit. `None` uses twice + /// `max_canvas_bytes`, matching the decoder's working-buffer allowance. Default `None`. pub max_decode_bytes: Option, /// Merge a frame identical to the one before into it (its delay added). Default off. pub merge_identical: bool, @@ -244,6 +244,11 @@ mod tests { assert_eq!((a.frames.len(), a.stop), (2, Stop::ByteLimit)); assert!(matches!(read_frames(&bytes, &FrameLimits { max_canvas_bytes: 23, ..FrameLimits::default() }), Err(FrameError::LimitExceeded(_)))); assert!(read_frames(&bytes, &FrameLimits { max_canvas_bytes: 24, ..FrameLimits::default() }).is_ok()); + // Canvas and frame buffer each need 24 bytes; decoder allocations share this budget. + assert!(matches!( + read_frames(&bytes, &FrameLimits { max_decode_bytes: Some(24), ..FrameLimits::default() }), + Err(FrameError::LimitExceeded(_)) + )); assert!(read_frames(&bytes, &FrameLimits { max_decode_bytes: Some(1), ..FrameLimits::default() }).is_err()); }