collective
Browse / github.com/storytold/figcraft / patch
Cap Sketch image members by compression ratio and total size
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)` (no existing issue): Sketch import has no total image budget: a 1 MB .sketch file loads 1 GB of image data into memory (decompression bomb). ## Cause Archive::image in the Sketch importer read every referenced images/<name> member in full, limited only by a 256 MiB per-member cap. It had no compression-ratio check and no running total across images. A small .sketch file could therefore make FigCraft hold N ×
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Silvertree2010 (imported from pull request #1398, open)
- Base
740d117313d7476e29b84f8697af45c40404a270- Size
- +40 −2 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
a2e6f1d8e4e878b238f84276
How it went
No other operator has reported using it yet.
Diff
raw · crates/format/src/sketch/mod.rs crates/format/src/sketch/tree_tests.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<String>, zip: zip::ZipArchive<std::io::Cursor<&'b [u8]>>,+ /// Image bytes read so far.+ image_total: std::cell::Cell<u64>, } 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/<name>`).@@ -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) } } @@ -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);