From 576d8e228ce8efd100cb1a2849c064f13955119f Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:46:33 +0100 Subject: [PATCH 1/2] Cap Sketch image members by compression ratio and total size Found while testing crates/format/src/sketch/mod.rs:241-249 (Archive::image, MAX_IMAGE at :69), called once per distinct imageRef from crates/format/src/rest/mod.rs:951 (Ctx::image_key) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/format/src/sketch/mod.rs | 28 ++++++++++++++++++++++++-- crates/format/src/sketch/tree_tests.rs | 14 +++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/format/src/sketch/mod.rs b/crates/format/src/sketch/mod.rs index 985541b16..cd0a68306 100644 --- a/crates/format/src/sketch/mod.rs +++ b/crates/format/src/sketch/mod.rs @@ -67,6 +67,15 @@ const MAX_JSON_TOTAL: u64 = 2 * 1024 * 1024 * 1024; const MAX_JSON_TOTAL: u64 = 512 * 1024 * 1024; /// Largest image member read. const MAX_IMAGE: u64 = 256 * 1024 * 1024; +/// Most bytes an image member may unpack to, per compressed byte (images are already compressed, +/// a zero-filled one packs a thousand times), with a floor for small members. +const MAX_IMAGE_RATIO: u64 = 100; +const MIN_IMAGE_ALLOWANCE: u64 = 16 * 1024 * 1024; +/// Most image bytes read from one file in all. +#[cfg(not(target_pointer_width = "32"))] +const MAX_IMAGE_TOTAL: u64 = 1024 * 1024 * 1024; +#[cfg(target_pointer_width = "32")] +const MAX_IMAGE_TOTAL: u64 = 256 * 1024 * 1024; /// Most pages read from one file. const MAX_PAGES: usize = 1000; @@ -182,6 +191,8 @@ pub(crate) struct Archive<'b> { /// Problems met while reading (pages skipped). pub warnings: Vec, zip: zip::ZipArchive>, + /// Image bytes read so far. + image_total: std::cell::Cell, } impl<'b> Archive<'b> { @@ -234,7 +245,7 @@ impl<'b> Archive<'b> { warnings.push(format!("the file has more than {MAX_PAGES} pages; the rest were skipped")); } let version = meta.get("version").and_then(Value::as_u64).unwrap_or(0); - Ok(Archive { document, version, pages, warnings, zip: z }) + Ok(Archive { document, version, pages, warnings, zip: z, image_total: std::cell::Cell::new(0) }) } /// The bytes of the image a layer references (`images/`). @@ -245,7 +256,20 @@ impl<'b> Archive<'b> { } // A clone shares the parsed directory and the bytes; the resolver only has shared access. let mut z = self.zip.clone(); - crate::read_member(&mut z, &format!("images/{name}"), MAX_IMAGE).ok() + let member = format!("images/{name}"); + let (size, packed) = { + let f = z.by_name(&member).ok()?; + (f.size(), f.compressed_size()) + }; + let allowed = MAX_IMAGE + .min(packed.saturating_mul(MAX_IMAGE_RATIO).max(MIN_IMAGE_ALLOWANCE)) + .min(MAX_IMAGE_TOTAL.saturating_sub(self.image_total.get())); + if size > allowed { + return None; + } + let data = crate::read_member(&mut z, &member, allowed).ok()?; + self.image_total.set(self.image_total.get() + data.len() as u64); + Some(data) } } diff --git a/crates/format/src/sketch/tree_tests.rs b/crates/format/src/sketch/tree_tests.rs index 5399a3df5..738f6e40b 100644 --- a/crates/format/src/sketch/tree_tests.rs +++ b/crates/format/src/sketch/tree_tests.rs @@ -158,6 +158,20 @@ fn oval_slice_and_bitmap() { assert_eq!(doc.images.len(), 1); } +#[test] +fn image_bombs_are_refused() { + // A valid 2×2 PNG followed by 40 MB of zeros packs to a few dozen kilobytes. + let mut png = Vec::new(); + image::RgbaImage::new(2, 2).write_to(&mut std::io::Cursor::new(&mut png), image::ImageFormat::Png).unwrap(); + png.resize(png.len() + 40 * 1024 * 1024, 0); + let mut bitmap = rect("Photo", 0.0, 0.0, 30.0, 20.0); + bitmap["_class"] = json!("bitmap"); + bitmap["image"] = json!({"_class": "MSJSONFileReference", "_ref_class": "MSImageData", "_ref": "images/bomb.png"}); + let (doc, _) = open_with(vec![artboard("A", "Board", 0.0, 0.0, 100.0, 100.0, vec![bitmap])], &[("images/bomb.png", &png)]); + let total: usize = doc.images.values().map(|i| i.bytes.len()).sum(); + assert!(total < 1024 * 1024, "{total} image bytes kept"); +} + #[test] fn unknown_class_is_a_warning() { let mut odd = rect("Odd", 0.0, 0.0, 10.0, 10.0); From 23e16be7f5de8f2f295ed0f0d64ec32dc98b6317 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 29 Oct 2026 09:36:10 +0700 Subject: [PATCH 2/2] #1398: share the member size check between JSON and images (capped_member) # Conflicts: # crates/format/src/sketch.rs --- crates/format/src/sketch.rs | 52 ++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/crates/format/src/sketch.rs b/crates/format/src/sketch.rs index 21cb7d2b2..8186ce26d 100644 --- a/crates/format/src/sketch.rs +++ b/crates/format/src/sketch.rs @@ -56,10 +56,10 @@ pub const NEWEST_TESTED_VERSION: u64 = 139; pub const TOO_OLD: &str = "saved by a Sketch older than 48; open it in a newer Sketch and save it again"; /// Largest JSON member read (a real page of a few thousand layers takes 33 MB). const MAX_JSON_MEMBER: u64 = 512 * 1024 * 1024; -/// Most bytes JSON members may unpack to, per compressed byte (real files pack 5–20 times; a -/// zero-filled array a thousand times), with a floor for small members. -const MAX_JSON_RATIO: u64 = 100; -const MIN_JSON_ALLOWANCE: u64 = 16 * 1024 * 1024; +/// Most bytes a member may unpack to, per compressed byte (real JSON packs 5–20 times and images +/// barely; a zero-filled member a thousand times), with a floor for small members. +const MAX_RATIO: u64 = 100; +const MIN_ALLOWANCE: u64 = 16 * 1024 * 1024; /// Most JSON read from one file in all. #[cfg(not(target_pointer_width = "32"))] const MAX_JSON_TOTAL: u64 = 2 * 1024 * 1024 * 1024; @@ -67,10 +67,6 @@ const MAX_JSON_TOTAL: u64 = 2 * 1024 * 1024 * 1024; const MAX_JSON_TOTAL: u64 = 512 * 1024 * 1024; /// Largest image member read. const MAX_IMAGE: u64 = figcraft_doc::ImageData::MAX_BYTES; -/// Most bytes an image member may unpack to, per compressed byte (images are already compressed, -/// a zero-filled one packs a thousand times), with a floor for small members. -const MAX_IMAGE_RATIO: u64 = 100; -const MIN_IMAGE_ALLOWANCE: u64 = 16 * 1024 * 1024; /// Most image bytes read from one file in all. #[cfg(not(target_pointer_width = "32"))] const MAX_IMAGE_TOTAL: u64 = 1024 * 1024 * 1024; @@ -256,37 +252,39 @@ impl<'b> Archive<'b> { } // A clone shares the parsed directory and the bytes; the resolver only has shared access. let mut z = self.zip.clone(); - let member = format!("images/{name}"); - let (size, packed) = { - let f = z.by_name(&member).ok()?; - (f.size(), f.compressed_size()) - }; - let allowed = MAX_IMAGE - .min(packed.saturating_mul(MAX_IMAGE_RATIO).max(MIN_IMAGE_ALLOWANCE)) - .min(MAX_IMAGE_TOTAL.saturating_sub(self.image_total.get())); - if size > allowed { - return None; - } - let data = crate::read_member(&mut z, &member, allowed).ok()?; - self.image_total.set(self.image_total.get() + data.len() as u64); - Some(data) + let mut total = self.image_total.get(); + let data = capped_member(&mut z, &format!("images/{name}"), MAX_IMAGE, &mut total, MAX_IMAGE_TOTAL).ok(); + self.image_total.set(total); + data } } -/// JSON member `name`, refused before unpacking when it would be larger than `limit`, than -/// [`MAX_JSON_RATIO`] times what it takes in the archive (past a floor), or than what is left of -/// the file's [`MAX_JSON_TOTAL`] (`total`: read so far). -fn json_member(z: &mut zip::ZipArchive>, name: &str, limit: u64, total: &mut u64) -> Result { +/// Member `name`, refused before unpacking when it would be larger than `limit`, than +/// [`MAX_RATIO`] times what it takes in the archive (past a floor), or than what is left of +/// `max_total` (`total`: read so far, counted up). +fn capped_member( + z: &mut zip::ZipArchive>, + name: &str, + limit: u64, + total: &mut u64, + max_total: u64, +) -> Result, FormatError> { let (size, packed) = { let f = z.by_name(name).map_err(|e| corrupt(format!("{name}: {e}")))?; (f.size(), f.compressed_size()) }; - let allowed = limit.min(packed.saturating_mul(MAX_JSON_RATIO).max(MIN_JSON_ALLOWANCE)).min(MAX_JSON_TOTAL.saturating_sub(*total)); + let allowed = limit.min(packed.saturating_mul(MAX_RATIO).max(MIN_ALLOWANCE)).min(max_total.saturating_sub(*total)); if size > allowed { return Err(corrupt(format!("{name} is too large"))); } let data = crate::read_member(z, name, allowed)?; *total += data.len() as u64; + Ok(data) +} + +/// JSON member `name`, read by [`capped_member`] within the file's [`MAX_JSON_TOTAL`]. +fn json_member(z: &mut zip::ZipArchive>, name: &str, limit: u64, total: &mut u64) -> Result { + let data = capped_member(z, name, limit, total, MAX_JSON_TOTAL)?; serde_json::from_slice(&data).map_err(|e| corrupt(format!("{name}: {e}"))) }