From d2e349ee6c46bd9990a7cdbc11633b4af7124a40 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Thu, 22 Oct 2026 17:58:16 +0700 Subject: [PATCH 01/34] README: the app icon above the title --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 1de756458a..538e34d047 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,10 @@

+

+ PhotoCraft icon: an engraved nine-tailed fox on a blue tile +

+

PhotoCraft

From f65832931b00e5fd27b384219baf10fd3edb4f53 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:43:52 -0400 Subject: [PATCH 02/34] security(compose): cap uncached pattern conversion When a pattern exceeded the preparation cache budget, every lookup still called Tile::new_in, expanding the whole sparse pattern into RGBA floats on each output tile (and possibly concurrently). The cache budget only controlled retained entries, not this conversion work. Reject patterns whose full RGBA conversion would exceed an absolute 256 MiB cap on the uncached path, so oversized patterns are skipped instead of re-expanded per tile. The cap is independent of the cache budget, so small patterns still convert on demand when nothing is cached. Finding: cc0b2f77-5578-48ed-aba0-1f3beccd21cd (CWE-400) --- crates/compose/src/pattern.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/compose/src/pattern.rs b/crates/compose/src/pattern.rs index d107ecf4e7..b66ab0ad1e 100644 --- a/crates/compose/src/pattern.rs +++ b/crates/compose/src/pattern.rs @@ -12,6 +12,10 @@ use photocraft_doc::{ColorMode, Pattern, PixelFormat, Rect, SampleType}; pub(crate) const PREPARED_PATTERN_BYTES: usize = 64 << 20; const MAX_PREPARED_PATTERNS: usize = 64; +/// Absolute cap on a single pattern's RGBA conversion on the uncached path. A pattern this +/// large cannot be cached, so re-expanding it for every output tile would be a DoS; above the +/// cap it is skipped instead. Independent of the cache budget so small patterns still convert. +const MAX_UNCACHED_PATTERN_BYTES: usize = 256 << 20; type PreparedSlot = Arc>>>; struct PreparedState { @@ -78,6 +82,13 @@ impl<'a> PreparedPatterns<'a> { // Run on the requesting worker to preserve its active CMYK profile. slot.get_or_init(|| Tile::new_in(p, self.mode).map(Arc::new)).clone() } + // Uncached path: a pattern whose full conversion would exceed an absolute cap is + // not materialized at all. Otherwise every output tile would re-expand the entire + // sparse pattern into RGBA floats (and could do so concurrently), exhausting CPU and + // memory even though cache admission already refused to retain it. The cap is + // independent of the (possibly zero) cache budget, so small patterns still convert + // on demand when nothing is cached. + None if bytes.is_none_or(|b| b > MAX_UNCACHED_PATTERN_BYTES) => None, None => Tile::new_in(p, self.mode).map(Arc::new), } } From 7016152f224c36f46d7d154ef1096918557f7fc6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:42:34 -0400 Subject: [PATCH 03/34] security(compose): bound dense mask working set build_mask derives its working area from sparse mask geometry grown by feather, so two distant changed pixels could span a canvas-sized rectangle. It then allocated f32 planes (coverage, pixel-mask values, blur scratch) over that area, gigabytes during even a one-pixel render. Check the estimated working bytes before the dense allocation and fall back to the constant default mask above 512 MiB, so pathological geometry degrades feather detail instead of exhausting memory. Finding: 8c04c5ef-a5bd-48f4-bfcf-83615bcf11c0 (CWE-770) --- crates/compose/src/masks.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/compose/src/masks.rs b/crates/compose/src/masks.rs index 802a0f2df9..7335be90d0 100644 --- a/crates/compose/src/masks.rs +++ b/crates/compose/src/masks.rs @@ -221,7 +221,14 @@ fn build_mask(layer: &Layer, canvas: Rect) -> Surface { } let area = area.intersect(&canvas); let mut s = Surface::with_default(FORMAT, &[p_def * v_out]); - if !area.is_empty() { + // Bound the transient dense working set: `area` is derived from sparse mask geometry grown + // by feather, so two distant changed pixels could span a canvas-sized rectangle. Building + // f32 planes over it (coverage, pixel-mask values, blur scratch) could be gigabytes during + // a one-pixel render. Above the budget, fall back to the constant default mask rather than + // allocating; feather detail is lost only for pathological geometry. + const MAX_MASK_WORKING_BYTES: u64 = 512 << 20; + let working = (area.width() as u64).saturating_mul(area.height() as u64).saturating_mul(std::mem::size_of::() as u64).saturating_mul(3); + if !area.is_empty() && working <= MAX_MASK_WORKING_BYTES { let (w, h) = (area.width() as usize, area.height() as usize); // A slot initializer must not enter Rayon: another tile requesting the // same mask could be stolen onto its stack and wait for its own build. From bd109515a91602939f5c955da8e73cc8dfc15737 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:41:08 -0400 Subject: [PATCH 04/34] security(io): bound XCF copied layer metadata The XCF importer reparses every layer offset and allocates a new owned name and item path per reference. Repeated offsets referencing one record with a large name or path could retain gigabytes of metadata, which the per-field and per-layer limits did not bound in aggregate. Add a document-wide metadata budget (32 MiB) to the Image reader and charge each copied layer name and item path against it, erroring when exhausted. Finding: 1631d2df-fe8f-4734-b14e-380db11afb26 (CWE-789) --- crates/io/src/xcf.rs | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/io/src/xcf.rs b/crates/io/src/xcf.rs index 15586a55dd..4bb62cacbb 100644 --- a/crates/io/src/xcf.rs +++ b/crates/io/src/xcf.rs @@ -42,6 +42,10 @@ pub(crate) const MAX_LAYERS: usize = 8192; const MAX_CHANNELS: usize = 1024; /// Group nesting. const MAX_DEPTH: usize = 64; +/// Total copied layer metadata (names and item paths). A single reused record with a large +/// name is bounded by MAX_STRING, but repeated layer offsets referencing it must not multiply +/// into unbounded retained metadata. +const MAX_METADATA_BYTES: usize = 32 << 20; /// A string field (layer names, parasite names). const MAX_STRING: usize = 1 << 20; /// A property payload or a parasite that is read into memory. @@ -381,6 +385,10 @@ struct Image<'a> { base_type: u32, colormap: Vec<[u8; 3]>, budget: usize, + /// Remaining allowance for copied layer metadata (names, item paths). Repeated layer + /// offsets referencing one record with a large name would otherwise allocate an owned + /// copy per reference, multiplying a small file into gigabytes of retained metadata. + meta_budget: usize, warnings: Warnings, /// Layers GIMP composites with "Clip to backdrop" (its default for every mode but Normal). clipped_to_backdrop: usize, @@ -685,7 +693,18 @@ pub(crate) fn import(name: &str, bytes: &[u8], ctl: &Interrupt<'_>) -> Result Image<'a> { let mut c = self.c.at(ptr); let (lw, lh, kind) = (c.u32()?, c.u32()?, c.u32()?); let name = c.string()?; + // Charge the owned name against the document-wide metadata budget: repeated layer + // offsets referencing one large name must not retain unbounded copies. + self.meta_budget = self.meta_budget.checked_sub(name.len()).ok_or_else(|| invalid("layer metadata exceeds the budget"))?; let lprops = props(&mut c)?; let hptr = c.ptr()?; let mptr = c.ptr()?; @@ -902,7 +924,11 @@ impl<'a> Image<'a> { 26 => text = true, 28 => locks.pixels = v != 0, 29 => group = true, - 30 => path = p.data.as_chunks::<4>().0.iter().map(|s| u32::from_be_bytes(*s)).collect(), + 30 => { + path = p.data.as_chunks::<4>().0.iter().map(|s| u32::from_be_bytes(*s)).collect(); + self.meta_budget = + self.meta_budget.checked_sub(path.len().saturating_mul(4)).ok_or_else(|| invalid("layer metadata exceeds the budget"))?; + } 31 => expanded = v & 1 != 0, 32 => locks.position = v != 0, 34 => tag = label(v), From 0e1a7a8b2a9533508e32cd86be3c4969233389c5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:39:17 -0400 Subject: [PATCH 05/34] security(io): bound PDN layer-name copies PDN conversion copied a referenced name into an owned String for every output layer, while the NRBF budget charged the shared string only once. Repeated references to one large name could multiply a small document into gigabytes of retained metadata. Cap each layer name at 64 KiB and the aggregate copied name bytes at 4 MiB, checked before each copy. Finding: fd77f4b2-39a2-48b5-b9d8-4ad69cf69469 (CWE-770) --- crates/io/src/pdn.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/io/src/pdn.rs b/crates/io/src/pdn.rs index 08e294b7bc..1e2d99f7af 100644 --- a/crates/io/src/pdn.rs +++ b/crates/io/src/pdn.rs @@ -154,6 +154,12 @@ pub(super) fn import(name: &str, bytes: &[u8], ctl: &Interrupt<'_>) -> Result) -> Result 255 { return Err(invalid("opacity exceeds 255")); } - let mut l = Layer::raster(graph.string(graph.field(props, "name")?)?, PixelFormat::RGBA8); + let mut l = Layer::raster( + { + let name = graph.string(graph.field(props, "name")?)?; + if name.len() > MAX_NAME_BYTES { + return Err(invalid("layer name is too long")); + } + name_bytes = name_bytes.saturating_add(name.len()); + if name_bytes > MAX_TOTAL_NAME_BYTES { + return Err(invalid("layer names exceed the total metadata budget")); + } + name.to_string() + }, + PixelFormat::RGBA8, + ); l.visible = graph.boolean(graph.field(props, "visible")?)?; l.opacity = opacity as f32 / 255.0; l.blend = blend(&graph, layer, props)?; From 8c0a9cb1851f5386b18d64601e7939b627a1c6b5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:36:53 -0400 Subject: [PATCH 06/34] security(raw): bound cumulative retained tile memory read_plane decodes every tile in parallel and retains all decoded tile buffers in before assembly copies only the useful pixels. Each tile is charged individually, but file-controlled padding and repeated source ranges can make the cumulative retained samples far exceed both the input and the useful image. Sum the padded tile samples (two bytes each) and reject the plane before decoding when that total exceeds max_alloc. Finding: e66981b1-0d7a-4738-a64c-37b5e8bf28d1 (CWE-770) --- crates/raw/src/sensor.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/raw/src/sensor.rs b/crates/raw/src/sensor.rs index f2cffdb808..cab5ce2a05 100644 --- a/crates/raw/src/sensor.rs +++ b/crates/raw/src/sensor.rs @@ -255,6 +255,19 @@ pub(crate) fn read_plane(t: &Tiff, ifd: &Ifd, limits: &Limits, layout: JpegLayou } limits.check(width as u64, height as u64, samples as u64 * 2)?; let segments = segments(t, ifd, width, height)?; + // The decoded tiles are all retained in `decoded` before assembly copies only the useful + // pixels. Each tile is charged individually below, but file-controlled padding and repeated + // source ranges can make the *cumulative* retained samples far exceed both the input and the + // useful image. Reject that up front: the sum of padded tile samples (two bytes each) must + // fit the allocation budget. + let mut total_tile_samples: u64 = 0; + for s in &segments { + let tile = (s.w as u64).saturating_mul(s.h as u64).saturating_mul(samples as u64); + total_tile_samples = total_tile_samples.saturating_add(tile); + } + if total_tile_samples.saturating_mul(2) > limits.max_alloc { + return Err(RawError::LimitExceeded(format!("{} bytes of retained raw tiles exceed {}", total_tile_samples.saturating_mul(2), limits.max_alloc))); + } let le = t.le; let tiled = t.tag_uint(ifd, tag::TILE_WIDTH).is_some(); let decoded: Vec>> = par::map(segments.len(), |i| { From 2e2faec1a35a9db7c9a0fe55c830ffdadfba0fee Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:36:01 -0400 Subject: [PATCH 07/34] security(raw): cap lossless-JPEG Huffman table builds Each DHT definition for table id 0..3 calls Huffman::new, which allocates and fills a ~48 KiB lookup pair. A stream repeating DHT definitions (and DNG tiles referencing such a stream) could rebuild tables thousands of times, multiplying header-processing cost and memory churn while producing little output. A real lossless JPEG uses a handful of Huffman tables; cap cumulative table builds per stream at 64 and reject streams that exceed it. Finding: 7a31b581-d1b7-4f9c-91e5-ce42b47db87f (CWE-400) --- crates/raw/src/ljpeg.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/crates/raw/src/ljpeg.rs b/crates/raw/src/ljpeg.rs index e9c42bdeb4..4ab32df8b8 100644 --- a/crates/raw/src/ljpeg.rs +++ b/crates/raw/src/ljpeg.rs @@ -299,6 +299,11 @@ fn parse(data: &[u8]) -> Result> { let mut pos = 2; let mut frame: Option = None; let mut comp_ids: Vec = Vec::new(); + // A lossless JPEG uses at most a handful of Huffman tables. Each Huffman::new allocates a + // ~48 KiB lookup pair and fills it, so a stream that repeats DHT definitions could rebuild + // tables thousands of times. Cap the cumulative builds to bound that header-processing work. + const MAX_HUFFMAN_BUILDS: usize = 64; + let mut huffman_builds = 0usize; let mut huff: [Option; 4] = [None, None, None, None]; let mut restart = 0usize; loop { @@ -355,6 +360,10 @@ fn parse(data: &[u8]) -> Result> { let total: usize = counts.iter().map(|&c| usize::from(c)).sum(); let vals = seg.get(p + 17..p + 17 + total).ok_or_else(|| RawError::malformed("short DHT"))?; if id < 4 { + huffman_builds += 1; + if huffman_builds > MAX_HUFFMAN_BUILDS { + return Err(RawError::malformed("lossless JPEG defines too many Huffman tables")); + } huff[id] = Some(Huffman::new(&counts, vals)?); } p += 17 + total; @@ -573,4 +582,26 @@ mod tests { let lossy = [0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x0B, 8, 0, 1, 0, 1, 1, 1, 0x11, 0]; assert!(matches!(decode(&lossy, usize::MAX), Err(RawError::Unsupported(_)))); } + + #[test] + fn too_many_huffman_tables_are_rejected() { + // A valid stream still decodes; the same stream with thousands of repeated DHT + // definitions injected after the SOI is rejected before rebuilding tables endlessly. + let (w, h) = (8, 8); + let v = samples(w * h, 12, 5); + let j = lj92_encode(&v, w, h, 1, 12, 1, None); + assert!(decode(&j, usize::MAX).is_ok()); + + // Minimal DHT for id 0 (one length-1 code, value 0): FF C4, len=0x0014, then + // tc_th, 16 count bytes, one value. + let mut dht = vec![0xFF, 0xC4, 0x00, 0x14, 0x00]; + dht.extend_from_slice(&[1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]); // counts + dht.push(0x00); // one value + let mut flooded = j[..2].to_vec(); // SOI + for _ in 0..5000 { + flooded.extend_from_slice(&dht); + } + flooded.extend_from_slice(&j[2..]); // the rest of the valid stream + assert!(matches!(decode(&flooded, usize::MAX), Err(RawError::Malformed(_)))); + } } From bf98617e2060acec5f8964aa977531977c4e8ee4 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:34:12 -0400 Subject: [PATCH 08/34] security(io): apply caller limits to RAW preview decoding upright_preview decoded a raw file's embedded JPEG with default codec options, discarding the caller's DevelopOptions.limits. A compressed embedded preview could therefore allocate and process far more pixel data than the caller authorized during the fallback and color-fit paths. Pass the caller's limits into upright_preview and translate them into the JPEG DecodeOptions, so both the Unsupported fallback and the preview look-fit respect the same policy. Finding: d17bc286-6765-41ce-bd41-579ee5b75cfa (CWE-409) --- crates/io/src/raw.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/io/src/raw.rs b/crates/io/src/raw.rs index 5ce761b521..820de8935b 100644 --- a/crates/io/src/raw.rs +++ b/crates/io/src/raw.rs @@ -35,8 +35,13 @@ fn limits() -> Limits { /// has one; otherwise the raw's IFD0 orientation applies (TIFF-based raws record it there, and /// their previews are usually stored as the sensor reads out). Developed raws are oriented by /// `photocraft-raw` and their EXIF carries no orientation, so nothing is turned twice. -fn upright_preview(raw: &[u8], jpeg: &[u8]) -> Result { - let img = codecs::decode_as_with(Format::Jpeg, jpeg, &codecs::DecodeOptions { keep_orientation: true, ..Default::default() })?; +fn upright_preview(raw: &[u8], jpeg: &[u8], limits: &photocraft_raw::Limits) -> Result { + // Decode the embedded preview under the caller's limits, not the codec defaults: an + // embedded JPEG can declare far more decoded pixels than the raw development the caller + // authorized, so the preview fallback must honor the same policy. + let codec_limits = + codecs::Limits { max_width: limits.max_width, max_height: limits.max_height, max_pixels: limits.max_pixels, max_alloc: limits.max_alloc }; + let img = codecs::decode_as_with(Format::Jpeg, jpeg, &codecs::DecodeOptions { keep_orientation: true, limits: codec_limits })?; let own = img.meta.exif.as_deref().map_or(1, codecs::exif_orientation); let o = if own != 1 { own } else { photocraft_raw::orientation(raw).unwrap_or_else(|| codecs::exif_orientation(raw)) }; Ok(img.oriented(o)?) @@ -216,7 +221,7 @@ pub fn fit_preview_look(raw: &[u8], sensor: &Sensor, opts: &DevelopOptions) -> R .min_by_key(|p| long(p)) .or_else(|| previews.iter().max_by_key(|p| long(p))) .ok_or("no embedded JPEG preview")?; - let img = upright_preview(raw, pick.jpeg).map_err(|e| format!("the embedded preview does not decode: {e}"))?; + let img = upright_preview(raw, pick.jpeg, &opts.limits).map_err(|e| format!("the embedded preview does not decode: {e}"))?; if img.layout() != ChannelLayout::Rgb || img.sample_type() != SampleType::U8 { return Err("the embedded preview is not 8-bit RGB".into()); } @@ -260,7 +265,7 @@ pub fn import_raw_with(name: &str, bytes: &[u8], opts: &DevelopOptions) -> Resul } Err(RawError::Unsupported(reason)) => match photocraft_raw::embedded_preview(bytes) { Some(p) => { - let img = upright_preview(bytes, p.jpeg)?; + let img = upright_preview(bytes, p.jpeg, &opts.limits)?; let mut r = image_to_document(name, &img)?; r.warnings.insert( 0, From 53bee4aa81406b4425a176ad9267f6e2b3878b41 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:31:27 -0400 Subject: [PATCH 09/34] security(raw): interpolate gain maps per pixel, not per full row GainMap::row materialized the entire horizontal gain grid (points_h floats) once per applicable image row, even though each output pixel only needs two neighboring points. File-controlled image height and horizontal map size multiplied into processing and allocation churn far beyond the input size. Add GainMap::at_rv, a direct four-corner bilinear lookup, and use it per output column in gain_row so no points_h-wide row is built per image row. A test proves at_rv matches the former row() + at() path. Finding: 7ebec8df-1a7a-4d33-915f-097f8e9be0e5 (CWE-400) --- crates/raw/src/develop.rs | 3 +-- crates/raw/src/opcodes.rs | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/raw/src/develop.rs b/crates/raw/src/develop.rs index 5d28508750..71a8384cd0 100644 --- a/crates/raw/src/develop.rs +++ b/crates/raw/src/develop.rs @@ -460,12 +460,11 @@ fn gain_row(s: &Sensor, y: usize, x0: usize, w: usize, k: usize) -> Option= m.plane.saturating_add(m.planes) || ay < m.top || ay >= m.bottom || !(ay - m.top).is_multiple_of(m.row_pitch) { continue; } - let row = m.row(rv, k - m.plane); let o = out.get_or_insert_with(|| vec![1.0; w]); for (i, f) in o.iter_mut().enumerate() { let x = x0 + i; if x >= a.x && m.covers(x - a.x, ay) { - *f *= m.at(&row, ((x - a.x) as f64 + 0.5) / a.width as f64); + *f *= m.at_rv(rv, k - m.plane, ((x - a.x) as f64 + 0.5) / a.width as f64); } } } diff --git a/crates/raw/src/opcodes.rs b/crates/raw/src/opcodes.rs index 9dc231ca61..6be6d45077 100644 --- a/crates/raw/src/opcodes.rs +++ b/crates/raw/src/opcodes.rs @@ -58,6 +58,27 @@ impl GainMap { a + (b - a) * t } + /// Bilinearly interpolates the gain at image-relative row `rv` and column `rh` for map + /// plane `mp`, touching only the four surrounding grid points. This avoids building the + /// whole `points_h`-wide row per image row, which made development work proportional to + /// image height times horizontal map size even when each output row needs few values. + #[inline] + pub fn at_rv(&self, rv: f64, mp: usize, rh: f64) -> f32 { + let mp = mp.min(self.map_planes.saturating_sub(1)); + let fv = ((rv - self.origin_v) / self.spacing_v).clamp(0.0, (self.points_v - 1) as f64); + let i0 = (fv.floor() as usize).min(self.points_v - 1); + let i1 = (i0 + 1).min(self.points_v - 1); + let tv = (fv - i0 as f64) as f32; + let fh = ((rh - self.origin_h) / self.spacing_h).clamp(0.0, (self.points_h - 1) as f64); + let j0 = (fh.floor() as usize).min(self.points_h - 1); + let j1 = (j0 + 1).min(self.points_h - 1); + let th = (fh - j0 as f64) as f32; + let g = |i: usize, j: usize| self.gains.get((i * self.points_h + j) * self.map_planes + mp).copied().unwrap_or(1.0); + let top = g(i0, j0) + (g(i0, j1) - g(i0, j0)) * th; + let bot = g(i1, j0) + (g(i1, j1) - g(i1, j0)) * th; + top + (bot - top) * tv + } + /// `true` if the map covers active-area pixel (`ax`, `ay`). #[inline] pub fn covers(&self, ax: usize, ay: usize) -> bool { @@ -216,4 +237,21 @@ mod tests { huge[4 + 16 + 32..4 + 16 + 40].copy_from_slice(&[0xFF; 8]); // points assert!(parse(&huge).0.is_empty()); } + + #[test] + fn at_rv_matches_row_then_at() { + // at_rv (direct 4-corner bilinear) must equal the old row() + at() path for a range of + // sample positions, including points outside the grid that clamp. + let b = list(&[(9, gain_map_params([0, 0, 10, 10], 1, &[[1.0, 2.0], [3.0, 4.5]]))]); + let (maps, _) = parse(&b); + let m = &maps[0]; + for &rv in &[-1.0, 0.0, 0.17, 0.5, 0.83, 1.0, 2.0] { + let row = m.row(rv, 0); + for &rh in &[-1.0, 0.0, 0.1, 0.5, 0.9, 1.0, 3.0] { + let want = m.at(&row, rh); + let got = m.at_rv(rv, 0, rh); + assert!((got - want).abs() < 1e-6, "rv={rv} rh={rh}: {got} vs {want}"); + } + } + } } From 163a0000a5dbbae209637ddd455bf8a17c4109f7 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:29:12 -0400 Subject: [PATCH 10/34] security(raw): position RW2 bit cursor in constant time compressed_row positioned the bit cursor by replaying every preceding byte within its page (skip % PAGE calls of get(8)). For narrow images this replay runs per row, so a small file could consume disproportionate CPU just setting up cursors. Add Bits::new_at, which sets the page position and remaining-bit count directly from the row offset in O(1), and use it in compressed_row. A unit test proves it matches the old byte-by-byte replay. Finding: 2a7d3d4b-90ab-4887-9dba-c3884e224990 (CWE-407) --- crates/raw/src/rw2.rs | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/crates/raw/src/rw2.rs b/crates/raw/src/rw2.rs index c50d32c09c..ac9ada4189 100644 --- a/crates/raw/src/rw2.rs +++ b/crates/raw/src/rw2.rs @@ -138,10 +138,20 @@ struct Bits<'a> { } impl<'a> Bits<'a> { + #[cfg(test)] fn new(buf: &'a [u8], base: usize) -> Self { Bits { buf, base, pos: 0, nbits: 0 } } + /// Positions the cursor `skip_in_page` bytes into the page at `base`, in constant time. + /// Equivalent to `new(buf, base)` followed by `skip_in_page` calls of `get(8)`: the first + /// read loads one page (`pos = PAGE`), and each byte skipped consumes 8 bits. Narrow images + /// repeated this replay per row, which made cursor setup quadratic. + fn new_at(buf: &'a [u8], base: usize, skip_in_page: usize) -> Self { + debug_assert!(skip_in_page < PAGE); + Bits { buf, base, pos: PAGE, nbits: PAGE as u32 * 8 - 8 * skip_in_page as u32 } + } + #[inline] fn get(&mut self, num: u32) -> u32 { if num > self.nbits { @@ -162,10 +172,7 @@ impl<'a> Bits<'a> { /// Decodes one row of the vendor-compressed stream. fn compressed_row(buf: &[u8], row_bytes: usize, row: usize, out: &mut [u16]) { let skip = row_bytes * row; - let mut bits = Bits::new(buf, skip / PAGE * PAGE); - for _ in 0..(skip % PAGE) { - bits.get(8); - } + let mut bits = Bits::new_at(buf, skip / PAGE * PAGE, skip % PAGE); let mut sh: i32 = 0; for group in out.as_chunks_mut::<14>().0 { let mut pred = [0i32; 2]; @@ -354,6 +361,29 @@ mod tests { assert_eq!(b.get(8), 0x55); } + #[test] + fn new_at_matches_byte_replay() { + // new_at must position the cursor identically to new() followed by skip_in_page + // get(8) calls, so the quadratic per-row replay can be removed safely. + let mut buf = vec![0u8; 0x8000]; + for (i, b) in buf.iter_mut().enumerate() { + *b = (i * 7 + 3) as u8; + } + for base in [0usize, PAGE, 2 * PAGE] { + for skip in [0usize, 1, 7, 100, PAGE - 1] { + let mut replay = Bits::new(&buf, base); + for _ in 0..skip { + replay.get(8); + } + let mut direct = Bits::new_at(&buf, base, skip); + for _ in 0..20 { + assert_eq!(replay.get(5), direct.get(5), "base {base} skip {skip}"); + assert_eq!(replay.get(8), direct.get(8), "base {base} skip {skip}"); + } + } + } + } + #[test] fn compressed_group_walks_predictors() { // One 14-pixel group: both parities start absolute, a delta of 0x80 From c5a6afcf68f4906257c4d717859a165431f44947 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:27:22 -0400 Subject: [PATCH 11/34] security(raw): reject tiled Sony cRAW and cap row references read_craw treats every segment as a run of full-width image rows and grew the row-reference vector before checking coverage. A tiled layout (which does not describe full-width rows) with repeated tile offsets could push far more than height rows, allocating gigabytes before the post-loop coverage check ran. Reject tiled cRAW layouts up front (cRAW is only stored as strips) and stop collecting rows once the image height is reached. Finding: 3c2601a1-ad92-45b6-96f7-be5da87d97eb (CWE-400) --- crates/raw/src/sony.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/raw/src/sony.rs b/crates/raw/src/sony.rs index e1961680df..b00fd0360a 100644 --- a/crates/raw/src/sony.rs +++ b/crates/raw/src/sony.rs @@ -114,11 +114,23 @@ pub(crate) fn read_craw(t: &Tiff, ifd: &Ifd, limits: &Limits) -> Result { } let lut = tone_lut(&t.tag_uints(ifd, SONY_TONE_CURVE)).ok_or_else(|| RawError::unsupported("Sony compressed ARW without a usable tone curve (0x7010)"))?; limits.check(width as u64, height as u64, 2)?; + // The cRAW reader treats every segment as a sequence of full-width image rows. A tiled + // layout does not describe full-width rows, so accepting one would let repeated tile + // offsets push far more than `height` row references before the coverage check below. + // cRAW is only stored as strips, so reject tiles outright. + if t.tag_uint(ifd, tag::TILE_WIDTH).is_some() || t.tag_uint(ifd, tag::TILE_LENGTH).is_some() { + return Err(RawError::unsupported("tiled Sony compressed ARW is not supported")); + } // One byte per pixel: locate every row in the strips. let mut rows: Vec<&[u8]> = Vec::with_capacity(height); for s in segments(t, ifd, width, height)? { let n = s.h.min(height.saturating_sub(s.y)); for r in 0..n { + // Never collect more rows than the image has; a malformed strip layout cannot + // grow the reference buffer past the image height. + if rows.len() >= height { + return Err(RawError::malformed("compressed ARW strips describe more rows than the image")); + } let at = s.offset.checked_add(r * width).ok_or_else(|| RawError::malformed("raw strip offset overflows"))?; if (r + 1) * width > s.len { return Err(RawError::malformed("compressed ARW strip is shorter than its rows")); From 1f2be700579871f50e71d549164caba5761caa80 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:26:09 -0400 Subject: [PATCH 12/34] security(raw): bound TIFF structure-dump output Tiff::dump appended each entry's full ASCII value with no per-value or total-output limit. Thousands of entries can reference one large aliased ASCII value, expanding a small diagnostic input into a multi-gigabyte String. Cap each ASCII preview to 256 bytes (truncated at a char boundary) and stop the listing once the accumulated output reaches 1 MiB, marking it truncated. Finding: d994ec7f-25dd-4ae7-9673-04adc0aa8a3c (CWE-400) --- crates/raw/src/tiff.rs | 52 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/raw/src/tiff.rs b/crates/raw/src/tiff.rs index 671ab11fbe..cfcadee730 100644 --- a/crates/raw/src/tiff.rs +++ b/crates/raw/src/tiff.rs @@ -328,13 +328,32 @@ pub(crate) fn orientation(v: Option) -> u16 { /// A human-readable listing of every IFD and entry (debugging aid). pub(crate) fn dump(data: &[u8]) -> String { use std::fmt::Write; + // Caps so a crafted TIFF (many entries aliasing one large ASCII value) cannot expand a + // small diagnostic input into a multi-gigabyte String. + const MAX_ASCII_PREVIEW: usize = 256; + const MAX_OUTPUT: usize = 1 << 20; let Some(t) = Tiff::new(data) else { return "not a TIFF-structured file".into() }; let mut s = String::new(); - for ifd in t.all_ifds() { + 'ifds: for ifd in t.all_ifds() { let _ = writeln!(s, "IFD @{}", ifd.offset); for e in &ifd.entries { + if s.len() >= MAX_OUTPUT { + s.push_str("… (output truncated)\n"); + break 'ifds; + } let preview: String = if e.typ == 2 { - t.ascii(e).unwrap_or_default() + let mut a = t.ascii(e).unwrap_or_default(); + if a.len() > MAX_ASCII_PREVIEW { + // Truncate at a char boundary at or below the cap (lossy UTF-8 can hold + // multi-byte replacement chars). + let mut cut = MAX_ASCII_PREVIEW; + while cut > 0 && !a.is_char_boundary(cut) { + cut -= 1; + } + a.truncate(cut); + a.push('…'); + } + a } else { let n = (e.count as usize).min(12); let v: Vec = (0..n).map(|i| t.float(e, i).map(|f| format!("{f}")).unwrap_or_else(|| "?".into())).collect(); @@ -401,4 +420,33 @@ mod tests { let ifd = t.ifd_at(8, 0).unwrap(); assert_eq!(t.float(ifd.get(50730).unwrap(), 0), None); } + + #[test] + fn dump_output_is_bounded_for_aliased_ascii() { + // Many ASCII entries all pointing at one large value must not expand the diagnostic + // dump without bound. Build an IFD whose entries alias a big ASCII blob. + const ENTRIES: usize = 4000; + const BLOB: usize = 100_000; + let header = 8u32; + let ifd_at = header; + let entry_count_bytes = 2; + let entries_bytes = ENTRIES * 12; + let next_ptr_bytes = 4; + let blob_at = ifd_at as usize + entry_count_bytes + entries_bytes + next_ptr_bytes; + let mut b = b"II*\0".to_vec(); + b.extend_from_slice(&ifd_at.to_le_bytes()); + b.extend_from_slice(&(ENTRIES as u16).to_le_bytes()); + for i in 0..ENTRIES { + b.extend_from_slice(&(1000 + i as u16).to_le_bytes()); // tag + b.extend_from_slice(&2u16.to_le_bytes()); // type ASCII + b.extend_from_slice(&(BLOB as u32).to_le_bytes()); // count + b.extend_from_slice(&(blob_at as u32).to_le_bytes()); // value offset (aliased) + } + b.extend_from_slice(&0u32.to_le_bytes()); // next IFD = none + b.resize(blob_at + BLOB, b'A'); + let out = dump(&b); + // Comfortably below entries * blob (~400 MB); the caps keep it near 1 MiB. + assert!(out.len() < 2 << 20, "dump output was {} bytes", out.len()); + assert!(out.contains("output truncated") || out.len() < 1 << 20); + } } From 9edcb97687aa15b8cee5b614fd0cb360f361f95f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:24:26 -0400 Subject: [PATCH 13/34] security(codecs): admit HEIF coded image size, not just transformed When transforms (orientation/clean-aperture crop) are applied, admission checked only the transformed output size. The delegated HEVC decoder allocates the full coded image before cropping, so a HEIF with a small visible aperture over a large coded image bypassed the limit policy. Always check the coded dimensions against the full limits before delegating, in addition to the transformed size. Note: the related SPS-driven and grid-tile allocations (findings 75048229 and 2db4c59e) occur inside the pinned heic-rs dependency from parameter sets the container does not expose; fully bounding those needs a decoder-side change and is tracked separately. Finding: ec5cec31-8a62-4963-8045-e3ec9d9e4931 (CWE-400) --- crates/codecs/src/codecs/heif.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/codecs/src/codecs/heif.rs b/crates/codecs/src/codecs/heif.rs index 31da309ca3..efd9d2073a 100644 --- a/crates/codecs/src/codecs/heif.rs +++ b/crates/codecs/src/codecs/heif.rs @@ -45,6 +45,13 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits, keep_orientation: bool) -> R let sample = if info.sixteen_bit { SampleType::U16 } else { SampleType::U8 }; let (w, h) = if keep_orientation { (info.coded_width, info.coded_height) } else { (info.width, info.height) }; limits.check(w, h, layout, sample)?; + // The delegated decoder allocates the full coded image before applying any clean-aperture + // crop or orientation transform, so the coded dimensions must pass the limit policy even + // when the displayed (transformed) size is small. Checking only the transformed size let a + // tiny visible image hide a large coded allocation. + if !keep_orientation && (info.coded_width, info.coded_height) != (w, h) { + limits.check(info.coded_width, info.coded_height, layout, sample)?; + } let options = photocraft_heif::Options { max_pixels: limits.max_pixels, apply_transforms: !keep_orientation }; let decoded = photocraft_heif::decode(bytes, &options).map_err(err)?; // The decoder re-reads the container; trust its output's shape, not the probe's. From 867caefb5c789f03b8c1ce5e7b44299115c2b043 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:22:41 -0400 Subject: [PATCH 14/34] security(codecs): admit deep-EXR output size and validate chunks first Two deep-EXR allocation gaps: - flatten() allocated the flattened output and the full display-window stage (nc f32 samples per pixel) without a limit check; the deep decode only admitted the data window at four bytes per pixel, before the channel layout was known. Pass Limits into flatten and re-check both buffers at the real f32 layout size. - decode_deep built a per-pixel usize block index before validating the chunk's declared packed sizes. Compute the block bounds first, validate the payload is present, then materialize the index, so a truncated or lying chunk is rejected before the index is allocated. Findings: - feb6fcd8-58df-47e2-a04a-d4d81d5565aa (CWE-789, display-stage allocation) - 9c53025d-65f8-4bcc-9161-143a4c5d7f46 (CWE-789, index before payload check) --- crates/codecs/src/codecs/deep_exr.rs | 41 ++++++++++++++++++++-------- crates/codecs/src/codecs/exr.rs | 2 +- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/crates/codecs/src/codecs/deep_exr.rs b/crates/codecs/src/codecs/deep_exr.rs index 29f3762ebc..4fe9def532 100644 --- a/crates/codecs/src/codecs/deep_exr.rs +++ b/crates/codecs/src/codecs/deep_exr.rs @@ -341,15 +341,17 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res return Err(err(format!("chunk declares part {part}, expected {header_index}"))); } } - // Block coordinates → the block's global pixel range and its slot. - let (block_pixels, slot, row_w): (Vec, usize, usize) = match &header.blocks { + // Block coordinates → the block's pixel bounds and its slot. The per-pixel index is + // built only after the chunk's declared payload sizes are validated below, so a + // truncated or lying chunk cannot drive a large index allocation first. + let (block_rows, block_cols, slot, row_w, scanline_row): (std::ops::Range, std::ops::Range, usize, usize, bool) = match &header.blocks { BlockDescription::ScanLines => { let y = i32(&mut cur)?; let row = match y.checked_sub(y0) { Some(d) if d >= 0 && (d as usize) < h => d as usize, _ => return Err(err(format!("scan line block y={y} is outside the data window"))), }; - ((0..w).map(|x| row * w + x).collect(), row, w) + (row..row + 1, 0..w, row, w, true) } BlockDescription::Tiles(_) => { let (tx, ty, lx, ly) = (i32(&mut cur)?, i32(&mut cur)?, i32(&mut cur)?, i32(&mut cur)?); @@ -362,16 +364,11 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res let (bx, by) = ((tx as usize) * tile_w, (ty as usize) * tile_h); let rows = (by + tile_h).min(h); let cols = (bx + tile_w).min(w); - let mut px = Vec::with_capacity((rows - by) * (cols - bx)); - for yy in by..rows { - for xx in bx..cols { - px.push(yy.saturating_mul(w).saturating_add(xx)); - } - } let slot = (ty as usize).saturating_mul(tiles_x) + tx as usize; - (px, slot, cols - bx) + (by..rows, bx..cols, slot, cols - bx, false) } }; + let block_len = block_rows.len().saturating_mul(block_cols.len()); if seen.get(slot).copied().unwrap_or(true) { return Err(err(format!("two chunks cover block {slot}"))); } @@ -386,6 +383,20 @@ pub(crate) fn decode_deep(meta: &MetaData, bytes: &[u8], limits: &Limits) -> Res if packed_table > remaining || packed_data > remaining.saturating_sub(packed_table) { return Err(err("deep chunk sizes point past the end of the file")); } + // The chunk's payload is present; now materialize the block's global pixel index. For + // scan lines this is one row; for tiles the clipped tile rectangle. + let block_pixels: Vec = if scanline_row { + let row = block_rows.start; + (0..w).map(|x| row * w + x).collect() + } else { + let mut px = Vec::with_capacity(block_len); + for yy in block_rows.clone() { + for xx in block_cols.clone() { + px.push(yy.saturating_mul(w).saturating_add(xx)); + } + } + px + }; let what = "the sample-count table"; let table_bytes = read_exact(&mut cur, packed_table as usize, what)?; let expected_table = block_pixels.len().checked_mul(4).ok_or_else(|| err("count table overflows"))?; @@ -552,7 +563,7 @@ fn header_end(bytes: &[u8]) -> Result { /// into straight alpha at the end. Without any alpha channel the front-most sample wins, /// opaque. Z, ZBack and other deep channels influence the flat result only through the /// ordering; volume samples (with ZBack) are approximated as point samples. -pub(crate) fn flatten(deep: &DeepImage) -> Result { +pub(crate) fn flatten(deep: &DeepImage, limits: &Limits) -> Result { let npx = deep.width as usize * deep.height as usize; let has = |n: &str| deep.channel(n).is_some(); let rgb = has("R") && has("G") && has("B"); @@ -569,6 +580,10 @@ pub(crate) fn flatten(deep: &DeepImage) -> Result { return Err(CodecError::unsupported(F, "no R/G/B or Y channels in the deep part")); }; let nc = layout.channels(); + // Admit the flattened output at its true size: nc f32 samples per pixel. The deep decode + // admitted the data window at four bytes per pixel, before the flattened channel layout + // was known, so re-check here with the actual layout and f32 sample size. + limits.check_bytes(deep.width, deep.height, nc as u64 * 4)?; let samples = |n: &str| -> &[f32] { deep.channel(n).map(|c| c.samples.as_slice()).unwrap_or(&[]) }; let (r, g, b) = (samples("R"), samples("G"), samples("B")); let (ar, ag, ab) = (samples("AR"), samples("AG"), samples("AB")); @@ -665,6 +680,10 @@ pub(crate) fn flatten(deep: &DeepImage) -> Result { // pads to the stage at its offset, transparent elsewhere (limits validated the size). let mut img = match deep.stage { Some((sw, sh)) => { + // The display stage can be larger than the data window; admit it at the real f32 + // layout size before allocating, since the earlier deep-decode check used four + // bytes per pixel and did not know the flattened channel count. + limits.check_bytes(sw, sh, nc as u64 * 4)?; let mut padded = vec![0f32; sw as usize * sh as usize * nc]; let row_len = deep.width as usize * nc; let dx = deep.origin.0.max(0) as usize; diff --git a/crates/codecs/src/codecs/exr.rs b/crates/codecs/src/codecs/exr.rs index 1b268ac559..2b32a2b7c4 100644 --- a/crates/codecs/src/codecs/exr.rs +++ b/crates/codecs/src/codecs/exr.rs @@ -164,7 +164,7 @@ pub(crate) fn decode(bytes: &[u8], limits: &Limits) -> Result // composite the samples into the flat image (a warning says what was lost). if meta.requirements.has_deep_data { let deep = super::deep_exr::decode_deep(&meta, bytes, limits)?; - return super::deep_exr::flatten(&deep); + return super::deep_exr::flatten(&deep, limits); } let pick = pick_part(&meta); From fed04ecef3ddca7e2058b96c37ec37e18ef50487 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:19:08 -0400 Subject: [PATCH 15/34] security(codecs): budget Cryptomatte per-pixel pair capacity The decode admission counted each pixel's channel samples and vector header but not the heap capacity of the per-pixel (id, coverage) pair lists. A compressed Cryptomatte document could therefore return pixel storage exceeding the caller's max_alloc budget. Include the maximum pair capacity (one pair per rank, 8 bytes each) in the per-pixel byte count before check_bytes. Finding: 5482ed4a-bc95-43fc-84d1-d5bfb6373761 (CWE-400) --- crates/codecs/src/codecs/exr_cryptomatte.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/codecs/src/codecs/exr_cryptomatte.rs b/crates/codecs/src/codecs/exr_cryptomatte.rs index 5f13f98621..91a5ede22d 100644 --- a/crates/codecs/src/codecs/exr_cryptomatte.rs +++ b/crates/codecs/src/codecs/exr_cryptomatte.rs @@ -217,14 +217,18 @@ pub(crate) fn decode(bytes: &[u8], layer_name: &str, limits: &Limits) -> Result< let too_large = || CodecError::malformed(F, "the layer is too large"); let (w32, h32) = (u32::try_from(w).map_err(|_| too_large())?, u32::try_from(h).map_err(|_| too_large())?); // Each pixel also gets a pair list, so count its header with the channel samples (four - // f32 channels per set: two ranks of ID and coverage). - let per_pixel = (layer.channels.len().max(1) as u64).saturating_mul(16).saturating_add(std::mem::size_of::>() as u64); + // f32 channels per set: two ranks of ID and coverage). Include the heap capacity the pair + // list can reach: up to one (id, coverage) pair per rank, 8 bytes each. Without this the + // returned per-pixel vectors could exceed the caller's max_alloc budget. + let ranks = layer.channels.len().saturating_mul(2); + let pair_capacity = (ranks as u64).saturating_mul(std::mem::size_of::<(f32, f32)>() as u64); + let per_pixel = + (layer.channels.len().max(1) as u64).saturating_mul(16).saturating_add(std::mem::size_of::>() as u64).saturating_add(pair_capacity); limits.check_bytes(w32, h32, per_pixel)?; let ex_layer = super::exr::read_layer(bytes, layer.part)?; // Two (ID, coverage) ranks per set: rank `slot * 2 + stream.rank`. - let ranks = layer.channels.len().saturating_mul(2); let mut ids: Vec> = vec![Vec::new(); ranks]; let mut coverages: Vec> = vec![Vec::new(); ranks]; for c in &ex_layer.channel_data.list { From 9e8a3eecf06a4ba70e29bede20786ce2cd7397ae Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:18:04 -0400 Subject: [PATCH 16/34] security(cms): bound inverse-curve and CLUT interpolation work Two per-pixel/per-sample amplifiers driven by a crafted ICC profile: - table_inverse recomputed a sampled curve's flat-run edge indices by scanning the whole table on every call, and transform construction inverts each destination curve OUT_TABLE times. Compute the edges once per curve via a new Curve::inverse_table batch method. - eval_multilinear enumerated 2^inputs cube corners over every declared input, including single-grid-point (degenerate) dimensions that contribute nothing. Interpolate over the effective dimensions only, so a 15-channel CLUT of one-point dims no longer does 2^15 iterations per pixel. Findings: - acfbe3a3-ee58-4b7e-b72a-5a4877dd15c9 (CWE-407, repeated flat-run scans) - 9fcc72e7-46d6-4f75-9333-cb14a65eeafd (CWE-407, degenerate-dim corner blowup) --- crates/cms/src/clut.rs | 27 +++++++++++++++++--- crates/cms/src/curve.rs | 49 +++++++++++++++++++++++++++++++++++-- crates/cms/src/transform.rs | 11 ++++----- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/crates/cms/src/clut.rs b/crates/cms/src/clut.rs index 186cd3f9f1..38992a2ed4 100644 --- a/crates/cms/src/clut.rs +++ b/crates/cms/src/clut.rs @@ -172,11 +172,16 @@ impl Clut { } let o = self.outputs; out[..o].iter_mut().for_each(|v| *v = 0.0); - for corner in 0..(1usize << n) { + // Only dimensions with more than one grid point contribute a cube axis. Degenerate + // dimensions have step 0 and ratio 0, so they always multiply the corner weight by 1 + // and never change the offset; enumerating 2^n corners over all declared inputs would + // do 2^(degenerate) redundant iterations. Interpolate over the effective dims only. + let eff: Vec = (0..n).filter(|&d| s[d] != 0).collect(); + for corner in 0..(1usize << eff.len()) { let mut w = 1.0f32; let mut off = base; - for d in 0..n { - if corner & (1 << d) != 0 { + for (bit, &d) in eff.iter().enumerate() { + if corner & (1 << bit) != 0 { w *= r[d]; off += s[d]; } else { @@ -245,4 +250,20 @@ mod tests { c.eval(&[0.5, 0.25], &mut o); assert!((o[0] - 0.75).abs() < 1e-6); } + + #[test] + fn degenerate_dimensions_do_not_change_the_result() { + // Dimensions with a single grid point contribute nothing, but the old code still + // enumerated 2^inputs corners over them. Interpolating over effective dims only must + // give the same answer as a reference built without the degenerate axes. + let f = |p: &[f32], o: &mut [f32]| o[0] = 0.25 * p[0] + 0.5 * p[2] + 0.125 * p[4]; + // Axes 1 and 3 have a single grid point (degenerate); axes 0, 2, 4 vary. + let c = Clut::sample(vec![3, 1, 4, 1, 5], 1, f); + let mut got = [0.0]; + for p in [[0.2f32, 0.0, 0.7, 0.0, 0.4], [1.0, 0.0, 0.0, 0.0, 1.0], [0.5, 0.0, 0.5, 0.0, 0.5]] { + c.eval_multilinear(&p, &mut got); + let want = 0.25 * p[0] + 0.5 * p[2] + 0.125 * p[4]; + assert!((got[0] - want).abs() < 1e-5, "got {} want {want}", got[0]); + } + } } diff --git a/crates/cms/src/curve.rs b/crates/cms/src/curve.rs index fe23e6e059..8b619842d1 100644 --- a/crates/cms/src/curve.rs +++ b/crates/cms/src/curve.rs @@ -99,6 +99,20 @@ impl Curve { (0..n).map(|i| self.eval64(i as f64 / (n - 1) as f64) as f32).collect() } + /// Inverse of the curve at each `y` in `ys`. For sampled-table curves the flat-run edge + /// indices are computed once for the whole batch instead of rescanning the table on every + /// call, so inverting a long constant table many times stays linear. + pub fn inverse_table(&self, ys: impl Iterator) -> Vec { + match self { + Curve::Table(t) if t.len() >= 2 => { + let increasing = t[t.len() - 1] >= t[0]; + let (edge_start, edge_end) = (edge_index(t, true), edge_index(t, false)); + ys.map(|y| table_inverse_with(t, y as f32, increasing, edge_start, edge_end)).collect() + } + _ => ys.map(|y| self.eval_inverse64(y) as f32).collect(), + } + } + /// `true` when the curve never decreases over `[0, 1]`. pub fn is_monotonic(&self) -> bool { let s = self.sample(1024); @@ -244,13 +258,24 @@ fn table_inverse(t: &[f32], y: f32) -> f32 { return y; } let increasing = t[n - 1] >= t[0]; + table_inverse_with(t, y, increasing, edge_index(t, true), edge_index(t, false)) +} + +/// [`table_inverse`] with the flat-run edge indices supplied. Computing those edges scans the +/// whole table, and transform construction inverts the same table thousands of times, so the +/// edges are computed once and reused across a batch (see [`Curve::inverse_table`]). +fn table_inverse_with(t: &[f32], y: f32, increasing: bool, edge_start: f32, edge_end: f32) -> f32 { + let n = t.len(); + if n < 2 { + return y; + } let (lo_v, hi_v) = if increasing { (t[0], t[n - 1]) } else { (t[n - 1], t[0]) }; if y <= lo_v { // Flat start: choose the last index still at the lowest value. - return if increasing { edge_index(t, true) } else { edge_index(t, false) }; + return if increasing { edge_start } else { edge_end }; } if y >= hi_v { - return if increasing { edge_index(t, false) } else { edge_index(t, true) }; + return if increasing { edge_end } else { edge_start }; } let (mut lo, mut hi) = (0usize, n - 1); while hi - lo > 1 { @@ -336,4 +361,24 @@ mod tests { let g = Curve::Gamma(2.2); assert!((g.eval64(-0.5) + 0.5f64.powf(2.2)).abs() < 1e-12); } + + #[test] + fn inverse_table_matches_per_call_including_flat_runs() { + // Batch inverse must equal calling eval_inverse64 per point. Include a table with flat + // runs at both ends (the case that used to rescan the table on every call). + let tables = [ + Curve::Table(vec![0.0, 0.0, 0.0, 0.25, 0.5, 1.0, 1.0, 1.0]), + Curve::Table(vec![0.0, 0.2, 0.4, 0.6, 0.8, 1.0]), + Curve::Table(vec![1.0, 0.5, 0.0]), + Curve::Gamma(2.2), + ]; + let ys: Vec = (0..=40).map(|i| i as f64 / 40.0).collect(); + for c in &tables { + let batch = c.inverse_table(ys.iter().copied()); + for (i, &y) in ys.iter().enumerate() { + let one = c.eval_inverse64(y) as f32; + assert!((batch[i] - one).abs() < 1e-6, "y={y}: batch {} vs per-call {one}", batch[i]); + } + } + } } diff --git a/crates/cms/src/transform.rs b/crates/cms/src/transform.rs index 04d11247f4..f30c17c82e 100644 --- a/crates/cms/src/transform.rs +++ b/crates/cms/src/transform.rs @@ -133,12 +133,11 @@ impl Transform { let tables = cs .iter() .map(|c| { - (0..OUT_TABLE) - .map(|i| { - let u = i as f64 / (OUT_TABLE - 1) as f64; - c.eval_inverse64(u * u * u * u).clamp(0.0, 1.0) as f32 - }) - .collect() + let ys = (0..OUT_TABLE).map(|i| { + let u = i as f64 / (OUT_TABLE - 1) as f64; + u * u * u * u + }); + c.inverse_table(ys).into_iter().map(|v| v.clamp(0.0, 1.0)).collect() }) .collect(); post = Some(Post { tables }); From 58cd22d4ff09a45357fc45b1563afd6a2b0a4632 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:15:37 -0400 Subject: [PATCH 17/34] security(doc): compute auto-slice coverage per band, not per cell auto_slices tested every grid cell against the full slice-rectangle list (covered(x, y) = taken.iter().any(...)). With slice edges forming the grid, that is cubic in the slice count, so a crafted document's slice arrangement could stall ordinary canvas drawing on the UI thread. For each row band, mark covered x-cells in a single pass over the rects that span the band, then scan the precomputed cells. This drops the inner work from O(rects) per cell to O(1), while producing the same tiling. Finding: 9fbea43b-3096-4cdc-b005-86d44a8fe57e (CWE-400) --- crates/doc/src/slices.rs | 77 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 6 deletions(-) diff --git a/crates/doc/src/slices.rs b/crates/doc/src/slices.rs index e22fcc0b79..7fb6ba3ad5 100644 --- a/crates/doc/src/slices.rs +++ b/crates/doc/src/slices.rs @@ -202,24 +202,42 @@ pub fn auto_slices(canvas: Rect, taken: &[Rect]) -> Vec { xs.dedup(); ys.sort_unstable(); ys.dedup(); - let covered = |x0: i32, y0: i32| taken.iter().any(|r| x0 >= r.x0 && x0 < r.x1 && y0 >= r.y0 && y0 < r.y1); - // Horizontal runs per row band. + // Clip the taken rectangles to the canvas once, so band coverage only considers rects that + // can actually cover a cell. + let clipped: Vec = taken.iter().map(|r| r.intersect(&canvas)).filter(|r| !r.is_empty()).collect(); + // Map an x edge to its cell index in `xs` (xs is sorted/deduped). + let x_cell = |x: i32| xs.partition_point(|&e| e <= x).saturating_sub(1); + // Horizontal runs per row band. For each band, mark covered x-cells in a single pass over + // the rects intersecting that band, instead of scanning every rect for every cell (which + // was cubic in the slice count and let a crafted document stall canvas drawing). + let ncells = xs.len().saturating_sub(1); let mut runs: Vec = Vec::new(); for yw in ys.windows(2) { + let (band_y0, band_y1) = (yw[0], yw[1]); + let mut covered_cell = vec![false; ncells]; + for r in &clipped { + if r.y0 <= band_y0 && band_y1 <= r.y1 { + // Rect spans this whole band vertically; mark the x-cells it covers. + let (c0, c1) = (x_cell(r.x0), x_cell(r.x1)); + for c in covered_cell.iter_mut().take(c1).skip(c0) { + *c = true; + } + } + } let mut start: Option = None; - for xw in xs.windows(2) { - let free = !covered(xw[0], yw[0]); + for (ci, xw) in xs.windows(2).enumerate() { + let free = !covered_cell.get(ci).copied().unwrap_or(false); match (free, start) { (true, None) => start = Some(xw[0]), (false, Some(s)) => { - runs.push(Rect::new(s, yw[0], xw[0], yw[1])); + runs.push(Rect::new(s, band_y0, xw[0], band_y1)); start = None; } _ => {} } } if let Some(s) = start { - runs.push(Rect::new(s, yw[0], *xs.last().unwrap_or(&canvas.x1), yw[1])); + runs.push(Rect::new(s, band_y0, *xs.last().unwrap_or(&canvas.x1), band_y1)); } } // Merge vertically adjacent runs with the same columns. @@ -299,6 +317,53 @@ mod tests { assert_eq!(auto_slices(canvas, &[]), vec![canvas]); } + #[test] + fn auto_slices_cover_exactly_with_many_overlapping_slices() { + // Several overlapping slices form a non-trivial grid. The per-band coverage pass must + // still tile the complement exactly: auto area + taken-union area == canvas, with no + // auto slice overlapping a taken rect or another auto slice. + let canvas = Rect::new(0, 0, 200, 150); + let taken = [Rect::new(10, 10, 90, 70), Rect::new(50, 40, 160, 120), Rect::new(120, 0, 200, 50), Rect::new(0, 100, 60, 150)]; + let auto = auto_slices(canvas, &taken); + // No auto slice intersects any taken rect. + for a in &auto { + for t in &taken { + assert!(a.intersect(t).is_empty(), "{a:?} overlaps taken {t:?}"); + } + } + // Auto slices do not overlap each other. + for (i, a) in auto.iter().enumerate() { + for b in &auto[i + 1..] { + assert!(a.intersect(b).is_empty(), "{a:?} overlaps {b:?}"); + } + } + // Auto area equals the canvas minus the union of taken rects (computed on the same grid). + let union_cells: i64 = { + // Sum cell areas covered by at least one taken rect, over the fine grid. + let mut xs = vec![canvas.x0, canvas.x1]; + let mut ys = vec![canvas.y0, canvas.y1]; + for t in &taken { + xs.extend([t.x0, t.x1]); + ys.extend([t.y0, t.y1]); + } + xs.sort_unstable(); + xs.dedup(); + ys.sort_unstable(); + ys.dedup(); + let mut sum = 0i64; + for yw in ys.windows(2) { + for xw in xs.windows(2) { + let covered = taken.iter().any(|r| xw[0] >= r.x0 && xw[0] < r.x1 && yw[0] >= r.y0 && yw[0] < r.y1); + if covered { + sum += i64::from(xw[1] - xw[0]) * i64::from(yw[1] - yw[0]); + } + } + } + sum + }; + assert_eq!(area(&auto) + union_cells, i64::from(canvas.width()) * i64::from(canvas.height())); + } + #[test] fn resolve_numbers_top_to_bottom() { let mut d = Document::new("site.psd", Size::new(100, 80), ColorMode::Rgb, SampleType::U8); From d0ad7ddb6fa085081d184b7d67ea5ce2b2e6ba7c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:13:56 -0400 Subject: [PATCH 18/34] security(psd): check padding length before scanning it FilterEffects::parse tested the trailing bytes with rest.iter().all(|b| b == 0) before checking rest.len() <= 3, so every loop iteration scanned the whole remaining buffer. A block with many empty sections produced quadratic CPU work. Reorder the short-circuiting && so the three-byte length check gates the zero scan, making each padding check constant work. Behavior is otherwise identical. Finding: 4887ae49-cfda-43b0-9296-a1816fc1ddba (CWE-407) --- crates/psd/src/filter_effects.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/psd/src/filter_effects.rs b/crates/psd/src/filter_effects.rs index e8b5efb698..cadb3ac8d0 100644 --- a/crates/psd/src/filter_effects.rs +++ b/crates/psd/src/filter_effects.rs @@ -170,7 +170,10 @@ impl FilterEffects { let pad = ((4 - len % 4) % 4).min(r.remaining() as u64); r.skip(pad as usize)?; let rest = r.peek_rest(); - if rest.iter().all(|&b| b == 0) && rest.len() <= 3 { + // Padding is at most three bytes. Check the length before scanning the suffix for + // zeros: otherwise each iteration scans the whole remaining buffer, which is + // quadratic when a block contains many empty sections. + if rest.len() <= 3 && rest.iter().all(|&b| b == 0) { break; } if rest.len() < 8 { From 7c07280b660ca937529a97f4e9176c79861e2667 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:13:17 -0400 Subject: [PATCH 19/34] security(affinity): bound raw child-slot traversal work Reader::children re-collected a group's child objects by scanning every raw Chld array slot, discarding nulls before the node budget counted anything. A shared group holding a large null-filled child array, linked many times, multiplied into excessive CPU work the MAX_NODES limit never observed. Charge every raw child slot (nulls included) against a document-wide MAX_CHILD_SLOTS budget before filtering, via a new Stream::array_len helper, and return a limit error when it is exhausted. Finding: 3973fcd0-acda-4312-b538-d986dcffeb1c (CWE-400) --- crates/affinity/src/access.rs | 9 +++++++++ crates/affinity/src/model.rs | 15 +++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/crates/affinity/src/access.rs b/crates/affinity/src/access.rs index 0b60ed7dbd..732feeef49 100644 --- a/crates/affinity/src/access.rs +++ b/crates/affinity/src/access.rs @@ -20,6 +20,15 @@ impl Stream { self.class(id) == Some(Tag::of(class)) } + /// The raw slot count of an object's array field (including null slots), for charging + /// traversal work before nulls are filtered out. `None` when the field is not an array. + pub fn array_len(&self, id: ObjId, tag: &[u8; 4]) -> Option { + match self.field(id, tag)? { + Value::Array(a) => Some(a.len()), + _ => None, + } + } + /// A child object (shared, linked or inline); `None` for null or missing. pub fn obj(&self, id: ObjId, tag: &[u8; 4]) -> Option { match self.field(id, tag)? { diff --git a/crates/affinity/src/model.rs b/crates/affinity/src/model.rs index 7c05072419..f77c970ae2 100644 --- a/crates/affinity/src/model.rs +++ b/crates/affinity/src/model.rs @@ -283,6 +283,12 @@ const MAX_TREE_DEPTH: usize = 128; /// Total layer nodes imported. const MAX_NODES: usize = 500_000; +/// Total raw `Chld` slots inspected across the whole traversal, including null slots and slots +/// reached through repeated references to a shared group. The node limit only counts produced +/// nodes, so a shared group holding a large null-filled child array could still be rescanned +/// many times; this bounds that work. Generous relative to MAX_NODES for real documents. +const MAX_CHILD_SLOTS: usize = 8_000_000; + /// Read a document: archive, `doc.dat` and the model. pub fn read(bytes: &[u8], limits: Limits) -> Result { let mut archive = Archive::open(bytes, limits)?; @@ -297,6 +303,7 @@ pub(crate) struct Reader<'s, 'a, 'b> { pub(crate) dpi: f64, warnings: BTreeMap, nodes: usize, + child_slots: usize, active: HashSet, pub(crate) geometry_budget: crate::geometry::Budget, /// Decoded pixels still available ([`Limits::max_pixel_bytes`] less what the layers read so @@ -312,6 +319,7 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { dpi: 72.0, warnings: BTreeMap::new(), nodes: 0, + child_slots: 0, active: HashSet::new(), geometry_budget: crate::geometry::Budget::default(), pixels_left: limits.max_pixel_bytes, @@ -383,6 +391,13 @@ impl<'s, 'a, 'b> Reader<'s, 'a, 'b> { let s = self.s; let mut out = Vec::new(); let mut mask = None; + // Charge every raw child slot (null slots included) against a document-wide budget + // before filtering. A shared group with a large null-filled Chld array could otherwise + // be rescanned through each reference without the node limit noticing. + self.child_slots = self.child_slots.saturating_add(s.array_len(parent, b"Chld").unwrap_or(0)); + if self.child_slots > MAX_CHILD_SLOTS { + return Err(Error::Limit("too many child slots in the layer tree")); + } for c in s.objs(parent, b"Chld") { if s.is(c, b"MRst") { // A mask layer inside a group or layer masks that container. From 29e555c2f655676c71c26ccea20e6371da9248b2 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:12:12 -0400 Subject: [PATCH 20/34] security(affinity): index live-corner radii by record subpath() looked up each polyline anchor's corner radius by scanning the entire radius list, O(anchors * radii). A file supplying large anchor and radius arrays could cause quadratic CPU work before geometry limits inspected the path. Build a record-to-maximum-radius map once and look each anchor up in it, preserving the previous fold(0.0, f64::max) behavior for duplicate record indices. Finding: 89f65d65-f637-44fc-9a81-a9039a413a4b (CWE-407) --- crates/affinity/src/geometry.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/affinity/src/geometry.rs b/crates/affinity/src/geometry.rs index 6c8ea602c9..6825abd511 100644 --- a/crates/affinity/src/geometry.rs +++ b/crates/affinity/src/geometry.rs @@ -210,7 +210,16 @@ fn anchors(records: &[Value]) -> Option> { fn subpath(anchors: &[Anchor], closed: bool, radii: Option<&Vec<(usize, f64)>>) -> Option { let first = anchors.first()?; if let Some(radii) = radii.filter(|r| !r.is_empty()) { - let corner = |a: &Anchor| radii.iter().filter(|(rec, _)| *rec == a.record).map(|(_, r)| *r).fold(0.0, f64::max); + // Index each record's maximum radius once. Scanning the whole radius list per anchor + // was O(anchors * radii), which a file with large anchor and radius arrays could drive + // into quadratic work. Duplicate record indices keep the maximum, matching the prior + // fold(0.0, f64::max) behavior. + let mut radius_by_record: std::collections::HashMap = std::collections::HashMap::with_capacity(radii.len()); + for &(rec, r) in radii { + let slot = radius_by_record.entry(rec).or_insert(0.0); + *slot = slot.max(r); + } + let corner = |a: &Anchor| radius_by_record.get(&a.record).copied().unwrap_or(0.0); let polyline = anchors.iter().all(|a| a.inh.is_none_or(|h| h == a.p) && a.out.is_none_or(|h| h == a.p)); if polyline { let mut pts: Vec<(Point, f64)> = anchors.iter().map(|a| (a.p, corner(a))).collect(); From 5adebc5d6df46a104547f1b9d872ee314ae1945c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:11:40 -0400 Subject: [PATCH 21/34] security(affinity): index object fields for O(1) lookup Object::get scanned the field vector linearly. An Affinity file can define one large shared object and reference it repeatedly as sibling layers, so repeated property lookups multiplied into quadratic CPU work without the fields appearing more than once in the file. Build a first-occurrence tag index when an object's fields are set (via Object::new and set_fields, the latter covering the root object that is filled after it is pushed). Positional fields (tag 0) stay out of the index, and first-match semantics are preserved. Finding: c315c4b0-dceb-459e-b39a-fa9c68ae6f40 (CWE-407) --- crates/affinity/src/stream.rs | 43 +++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/crates/affinity/src/stream.rs b/crates/affinity/src/stream.rs index a755747c45..2ffe12b7e5 100644 --- a/crates/affinity/src/stream.rs +++ b/crates/affinity/src/stream.rs @@ -73,11 +73,40 @@ pub struct Object { pub chain: Vec, /// Fields in stream order; positional fields have tag 0. pub fields: Vec<(Tag, Value)>, + /// First-occurrence index of each non-positional tag, so `get` is O(1). A shared object + /// can be referenced by many sibling layers, and each reference used to rescan all of its + /// fields; indexing once keeps repeated lookups from multiplying into quadratic work. + index: std::collections::HashMap, } impl Object { + pub fn new(class: Tag, kind: Kind, chain: Vec, fields: Vec<(Tag, Value)>) -> Self { + let mut index = std::collections::HashMap::with_capacity(fields.len()); + for (i, (t, _)) in fields.iter().enumerate() { + // Positional fields share tag 0 and are not looked up by tag; keep first-match + // semantics for real tags. + if t.0 != 0 { + index.entry(*t).or_insert(i); + } + } + Object { class, kind, chain, fields, index } + } + pub fn get(&self, tag: Tag) -> Option<&Value> { - self.fields.iter().find(|(t, _)| *t == tag).map(|(_, v)| v) + self.index.get(&tag).map(|&i| &self.fields[i].1) + } + + /// Replaces the fields (the root object is filled in after it is pushed) and rebuilds the + /// tag index so `get` stays correct. + fn set_fields(&mut self, fields: Vec<(Tag, Value)>) { + self.index.clear(); + self.index.reserve(fields.len()); + for (i, (t, _)) in fields.iter().enumerate() { + if t.0 != 0 { + self.index.entry(*t).or_insert(i); + } + } + self.fields = fields; } } @@ -124,10 +153,10 @@ pub fn parse(bytes: &[u8]) -> Result { c.u32()?; } let mut p = Parser { c, objects: Vec::new(), shared: std::collections::HashMap::new(), depth: 0, values: 0 }; - let root = p.push(Object { class, kind: Kind::Root, chain: vec![class], fields: Vec::new() })?; + let root = p.push(Object::new(class, Kind::Root, vec![class], Vec::new()))?; let fields = p.fields(true, class)?; if let Some(o) = p.objects.get_mut(root) { - o.fields = fields; + o.set_fields(fields); } Ok(Stream { root, objects: p.objects }) } @@ -252,7 +281,7 @@ impl Parser<'_> { } 0x30 => { let fields = self.fields(false, class)?; - let id = self.push(Object { class, kind: Kind::Positional, chain: Vec::new(), fields })?; + let id = self.push(Object::new(class, Kind::Positional, Vec::new(), fields))?; Value::Obj(Some(id)) } 0x31 | 0x32 if !tagged => return Err(Error::Malformed("object without a field tag")), @@ -350,7 +379,7 @@ impl Parser<'_> { } }; let fields = self.fields(true, class)?; - let id = self.push(Object { class, kind: Kind::Inline, chain: vec![class], fields })?; + let id = self.push(Object::new(class, Kind::Inline, vec![class], fields))?; Ok(Value::Obj(Some(id))) } @@ -369,7 +398,7 @@ impl Parser<'_> { return Err(Error::Malformed("object defined twice")); } // Register before the fields: an object's own fields may refer back to it. - let id = self.push(Object { class: Tag(0), kind: Kind::Shared, chain: Vec::new(), fields: Vec::new() })?; + let id = self.push(Object::new(Tag(0), Kind::Shared, Vec::new(), Vec::new()))?; self.shared.insert(key, id); let mut chain = Vec::new(); let mut fields = Vec::new(); @@ -395,7 +424,7 @@ impl Parser<'_> { let class = chain.first().copied().unwrap_or(Tag(0)); fields.extend(self.fields(true, class)?); if let Some(o) = self.objects.get_mut(id) { - *o = Object { class, kind: Kind::Shared, chain, fields }; + *o = Object::new(class, Kind::Shared, chain, fields); } Ok(Value::Obj(Some(id))) } From ef6092b12506d5c4587445c7590721d1b0c11e21 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:24:39 -0400 Subject: [PATCH 22/34] security(xtask): validate corpus paths and bound symlink traversal Row 16 (9f6a3c79): preserve platform filename semantics in corpus scans (no backslash-to-separator rewrite on Unix) and enforce destination containment before copying files into the corpus directory. Row 20 (6e2a985b): skip directory symlinks, dedupe visited directories by canonical identity, and cap total directories/files so symlink cycles can no longer drive exponential traversal. Fixes findings 9f6a3c79-8bdf-4751-8eb4-87d3154074c9 and 6e2a985b-6b14-4b97-af07-7ca8333b9b75. --- xtask/src/pinned.rs | 175 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 168 insertions(+), 7 deletions(-) diff --git a/xtask/src/pinned.rs b/xtask/src/pinned.rs index d480051599..baf798f1de 100644 --- a/xtask/src/pinned.rs +++ b/xtask/src/pinned.rs @@ -71,13 +71,53 @@ impl PinnedCorpus { .collect() } + /// Upper bound on directories visited and files collected during a single `collect` walk. + /// A corpus clone is operator-selected but can still contain attacker-planted symlinks; these + /// budgets keep a malformed tree from expanding into an unbounded traversal before the + /// manifest check runs. Real corpora are a few thousand files at most. + const MAX_DIRS: usize = 100_000; + const MAX_FILES: usize = 1_000_000; + fn collect(&self, dir: &Path, base: &Path, out: &mut Vec) { + let mut visited = std::collections::HashSet::new(); + let mut dirs = 0usize; + self.collect_inner(dir, base, out, &mut visited, &mut dirs); + } + + /// Walks `dir`, skipping directory symlinks and directories already seen through another + /// alias, and stops once the traversal or file budget is reached. `visited` holds resolved + /// directory identities (canonical paths) so two links to the same ancestor cannot make the + /// walk revisit it. + fn collect_inner(&self, dir: &Path, base: &Path, out: &mut Vec, visited: &mut std::collections::HashSet, dirs: &mut usize) { + if out.len() >= Self::MAX_FILES || *dirs >= Self::MAX_DIRS { + return; + } + // Record this directory by its resolved identity; bail if we have already walked it + // (a symlink cycle) or cannot resolve it. + match std::fs::canonicalize(dir) { + Ok(real) => { + if !visited.insert(real) { + return; + } + } + Err(_) => return, + } + *dirs += 1; let Ok(rd) = std::fs::read_dir(dir) else { return }; for e in rd.flatten() { + if out.len() >= Self::MAX_FILES || *dirs >= Self::MAX_DIRS { + return; + } let p = e.path(); - if p.is_dir() { - self.collect(&p, base, out); - } else if p.extension().and_then(|e| e.to_str()).is_some_and(|e| self.exts.iter().any(|x| e.eq_ignore_ascii_case(x))) { + // Use the entry's own metadata (do not follow links) to classify it. A directory + // symlink is skipped entirely so aliases to ancestors cannot drive the recursion. + let Ok(ft) = e.file_type() else { continue }; + if ft.is_symlink() { + continue; + } + if ft.is_dir() { + self.collect_inner(&p, base, out, visited, dirs); + } else if ft.is_file() && p.extension().and_then(|e| e.to_str()).is_some_and(|e| self.exts.iter().any(|x| e.eq_ignore_ascii_case(x))) { out.push(p.strip_prefix(base).unwrap_or(&p).to_path_buf()); } } @@ -89,7 +129,11 @@ impl PinnedCorpus { self.collect(dir, dir, &mut found); let mut out = Vec::new(); for f in found { - let rel = f.to_string_lossy().replace('\\', "/"); + // Build the manifest-relative path from the path's own components, joined with `/`. + // This preserves platform filename semantics: a Unix file literally named `a\b` keeps + // its backslash instead of being split into a fake `a/b` traversal component. Reject + // anything that is not a sequence of plain names (absolute roots, `..`, etc.). + let rel = rel_manifest_path(&f).ok_or_else(|| format!("unsafe corpus path {}", f.display()))?; let bytes = std::fs::read(dir.join(&f)).map_err(|e| format!("read {rel}: {e}"))?; out.push((sha256::hex(&bytes), format!("{prefix}{rel}"))); } @@ -144,12 +188,14 @@ impl PinnedCorpus { } }; for (_, file) in files { + let to = safe_dest_join(&dest, file).ok_or_else(|| format!("{file}: escapes corpus destination"))?; let from = locate(file).ok_or_else(|| format!("{file}: no upstream"))?; - place(&from, &dest.join(file))?; + place(&from, &to)?; } - for (from, to) in extras { + for (from, to_rel) in extras { if from.is_file() { - place(from, &dest.join(to))?; + let to = safe_dest_join(&dest, to_rel).ok_or_else(|| format!("{to_rel}: escapes corpus destination"))?; + place(from, &to)?; } } std::fs::write(dest.join("SOURCES.md"), (self.sources_md)(self)).map_err(|e| format!("write SOURCES.md: {e}"))?; @@ -269,6 +315,49 @@ impl PinnedCorpus { } } +/// Builds a `/`-joined manifest-relative path from a scanned relative path, using only the path's +/// own `Normal` components. Returns `None` if any component is a root, prefix, current-dir, or +/// parent-dir (`..`) element, so a scanned entry can never introduce traversal. Literal +/// backslashes inside a single Unix filename are preserved (they are part of the name, not a +/// separator). +fn rel_manifest_path(rel: &Path) -> Option { + use std::path::Component; + let mut parts = Vec::new(); + for comp in rel.components() { + match comp { + Component::Normal(os) => parts.push(os.to_str()?.to_string()), + // Root, prefix, `.` and `..` are all rejected: a corpus entry must be a plain, + // forward relative path beneath the scan root. + _ => return None, + } + } + if parts.is_empty() { + return None; + } + Some(parts.join("/")) +} + +/// Joins a manifest-relative corpus path onto `dest`, guaranteeing the result stays within +/// `dest`. Rejects absolute paths, parent-directory components, and any entry that would resolve +/// outside the destination. Returns the concrete target path on success. +fn safe_dest_join(dest: &Path, rel: &str) -> Option { + use std::path::Component; + let rel_path = Path::new(rel); + let mut out = dest.to_path_buf(); + for comp in rel_path.components() { + match comp { + Component::Normal(os) => out.push(os), + // Any non-plain component (root, prefix, `.`, `..`) could escape the destination. + _ => return None, + } + } + // The join must have added at least one component and must remain under `dest`. + if out == *dest || !out.starts_with(dest) { + return None; + } + Some(out) +} + /// Hex sha256 of a corpus file (reused by every corpus fetch, `xtask/pixls.sha256` included). fn file_hex(path: &std::path::Path) -> Option { std::fs::read(path).ok().map(|b| crate::sha256::hex(&b)) @@ -317,3 +406,75 @@ pub fn fetch_pixls(update: bool) -> Result<(), String> { println!("pixls: {} files verified in {}", crate::corpus_pins::PIXLS_FILES.len(), dest.display()); Ok(()) } + +#[cfg(test)] +mod tests { + use super::{rel_manifest_path, safe_dest_join}; + use std::path::{Path, PathBuf}; + + #[test] + fn rel_manifest_joins_normal_components_with_forward_slash() { + assert_eq!(rel_manifest_path(Path::new("a/b/c.psd")).as_deref(), Some("a/b/c.psd")); + assert_eq!(rel_manifest_path(Path::new("file.psd")).as_deref(), Some("file.psd")); + } + + #[test] + #[cfg(unix)] + fn rel_manifest_preserves_literal_backslash_in_unix_filename() { + // On Unix a backslash is an ordinary filename character, not a separator. The old code + // turned `weird\name.psd` into `weird/name.psd`, inventing a path component that the + // installer would then treat as a subdirectory. The component must be kept intact. + let rel = Path::new("weird\\name.psd"); + assert_eq!(rel_manifest_path(rel).as_deref(), Some("weird\\name.psd")); + } + + #[test] + fn rel_manifest_rejects_traversal_and_absolute() { + assert_eq!(rel_manifest_path(Path::new("../escape.psd")), None); + assert_eq!(rel_manifest_path(Path::new("a/../../escape.psd")), None); + assert_eq!(rel_manifest_path(Path::new("/abs/path.psd")), None); + assert_eq!(rel_manifest_path(Path::new("")), None); + } + + #[test] + fn safe_dest_join_contains_normal_paths() { + let dest = PathBuf::from("/corpus/psd"); + let got = safe_dest_join(&dest, "sub/dir/file.psd").expect("normal path allowed"); + assert!(got.starts_with(&dest)); + assert!(got.ends_with("sub/dir/file.psd")); + } + + #[test] + fn safe_dest_join_rejects_escapes() { + let dest = PathBuf::from("/corpus/psd"); + assert_eq!(safe_dest_join(&dest, "../../etc/passwd"), None); + assert_eq!(safe_dest_join(&dest, "/etc/passwd"), None); + assert_eq!(safe_dest_join(&dest, "a/../../b"), None); + // An empty relative path would resolve to `dest` itself, not a file within it. + assert_eq!(safe_dest_join(&dest, ""), None); + } + + #[test] + #[cfg(unix)] + fn collect_skips_directory_symlinks_and_cycles() { + use std::os::unix::fs::symlink; + + let tmp = std::env::temp_dir().join(format!("pinned-collect-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&tmp); + std::fs::create_dir_all(tmp.join("real")).unwrap(); + std::fs::write(tmp.join("real/a.psd"), b"data").unwrap(); + // A symlink back to an ancestor: the old collector would follow it and recurse forever. + symlink(&tmp, tmp.join("real/loop")).unwrap(); + // A symlink to a sibling directory also must not be descended into. + symlink(tmp.join("real"), tmp.join("alias")).unwrap(); + + let corpus = + super::PinnedCorpus { name: "t", dest: "t", upstreams: &[], manifest: "t.sha256", exts: &["psd"], subset: false, sources_md: |_| String::new() }; + let mut out = Vec::new(); + corpus.collect(&tmp, &tmp, &mut out); + // Exactly one real file is found; symlinked directories are skipped (no cycle, no dupes). + assert_eq!(out, vec![PathBuf::from("real/a.psd")]); + + let _ = std::fs::remove_dir_all(&tmp); + } +} From 2c02d06392fae1aeeac1360c7925a28a19c25e19 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:05:47 -0400 Subject: [PATCH 23/34] security(engine): count GIF frame delays with a map fps_from_delays counted each frame's delay by scanning a growing vector with iter_mut().find(), O(n^2) for a GIF with many distinct delays. A crafted animation within the frame limit could stall the synchronous import path. Count delays in a HashMap and keep a first-seen order vector so the most-common delay still wins, breaking ties toward the earliest-seen delay exactly as before. Finding: 23d54421-8e26-41b9-9690-ae1125ee2f85 (CWE-407) --- crates/engine/src/video_cmds.rs | 24 ++++++++++++++++-------- crates/engine/src/video_cmds/tests.rs | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/video_cmds.rs b/crates/engine/src/video_cmds.rs index 0d27d3c827..ddee127f2b 100644 --- a/crates/engine/src/video_cmds.rs +++ b/crates/engine/src/video_cmds.rs @@ -227,16 +227,24 @@ struct Footage { /// do, since they would otherwise play at an unintended 50-100 fps. fn fps_from_delays(delays: &[u32]) -> Option { let clean = |d: u32| if d < 20 { 100 } else { d }; - let mut counts: Vec<(u32, usize)> = Vec::new(); + // Count delays with a map keyed on the cleaned delay, recording first-seen order + // separately. A linear scan per frame (iter_mut().find) was O(n^2) for a GIF with many + // distinct delays. order[] preserves the tie-break: the earliest-seen delay wins. + let mut counts: std::collections::HashMap = std::collections::HashMap::new(); + let mut order: Vec = Vec::new(); for d in delays.iter().copied().map(clean) { - match counts.iter_mut().find(|(k, _)| *k == d) { - Some((_, n)) => *n += 1, - None => counts.push((d, 1)), - } + let n = counts.entry(d).or_insert_with(|| { + order.push(d); + 0 + }); + *n += 1; } - let best = counts.iter().fold(None::<(u32, usize)>, |best, &(d, n)| match best { - Some((_, bn)) if bn >= n => best, - _ => Some((d, n)), + let best = order.iter().fold(None::<(u32, usize)>, |best, &d| { + let n = counts[&d]; + match best { + Some((_, bn)) if bn >= n => best, + _ => Some((d, n)), + } }); best.map(|(d, _)| 1000.0 / d as f32) } diff --git a/crates/engine/src/video_cmds/tests.rs b/crates/engine/src/video_cmds/tests.rs index 3cdda8adc0..c97eb3b18d 100644 --- a/crates/engine/src/video_cmds/tests.rs +++ b/crates/engine/src/video_cmds/tests.rs @@ -579,3 +579,19 @@ fn render_video_rejects_gif_frames_too_large_to_stage() { let err = s.execute("file.export.renderVideo", json!({"dir": tmpdir("large-gif"), "format": "gif"})).unwrap_err(); assert!(err.to_string().contains("too large"), "{err}"); } + +#[test] +fn fps_from_delays_picks_most_common_and_breaks_ties_by_first_seen() { + // Most common delay wins: 100 ms appears twice, 40 ms once -> 10 fps. + assert_eq!(fps_from_delays(&[100, 40, 100]), Some(10.0)); + // Tie between 40 ms (25 fps) and 50 ms (20 fps): the first seen (40) wins. + assert_eq!(fps_from_delays(&[40, 50]), Some(25.0)); + // Delays under 20 ms are treated as 100 ms. + assert_eq!(fps_from_delays(&[0, 10, 5]), Some(10.0)); + assert_eq!(fps_from_delays(&[]), None); + // Many distinct delays used to be quadratic; this must stay fast and correct. Delay 20 + // (the one appearing twice) wins at 50 fps. + let mut many: Vec = (20..5_020).collect(); + many.push(20); + assert_eq!(fps_from_delays(&many), Some(50.0)); +} From ac7eca536aa8c9ac0eee622139511307aa3d8482 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:03:50 -0400 Subject: [PATCH 24/34] security(engine): convert spelling offsets in one pass The spelling list converted each misspelling's start and end byte offset to a character offset with char_at, which recounts the text prefix from the start every call. A layer with many short misspellings therefore caused quadratic rescanning on the synchronous UI thread. Collect the ascending offsets and map them to character offsets in a single forward walk over the text. char_at is removed (byte_at still handles the reverse direction in change()). Finding: 392eb136-b836-4400-8ed7-f1e4549ff0ae (CWE-407) --- crates/engine/src/type_spell_cmds.rs | 52 ++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/type_spell_cmds.rs b/crates/engine/src/type_spell_cmds.rs index 12a63a364c..c544634e5a 100644 --- a/crates/engine/src/type_spell_cmds.rs +++ b/crates/engine/src/type_spell_cmds.rs @@ -27,8 +27,30 @@ fn byte_at(text: &str, ci: usize) -> usize { text.char_indices().nth(ci).map_or(text.len(), |(b, _)| b) } -fn char_at(text: &str, bi: usize) -> usize { - text[..bi.min(text.len())].chars().count() +/// Maps ascending byte offsets to character offsets in a single forward pass. Calling +/// `char_at` per offset rescans the text prefix each time, which is quadratic when a layer +/// has many misspellings; this walks the text once for the whole batch. `offsets` must be +/// sorted ascending (misspelling start/end pairs are, since words are found left to right). +fn char_offsets(text: &str, offsets: &[usize]) -> Vec { + // (byte offset, char index) for each char boundary, plus the end-of-text boundary. + let mut boundaries = text.char_indices().map(|(b, _)| b); + let mut char_index = 0usize; + let mut current = boundaries.next(); + let mut result = Vec::with_capacity(offsets.len()); + for &target in offsets { + let clamped = target.min(text.len()); + // Advance to the first boundary at or past `clamped`; byte offsets from char + // boundaries land exactly on one, so the char index there is the answer. + while let Some(b) = current { + if b >= clamped { + break; + } + char_index += 1; + current = boundaries.next(); + } + result.push(char_index); + } + result } /// Type layers top to bottom (Layers panel order), skipping fully locked ones. @@ -76,10 +98,17 @@ fn list(s: &Session, p: &Value) -> Result { let mut out = Vec::new(); for id in layers_param(s, p) { let Some(t) = text(&st.doc, id) else { continue }; - for m in dict.misspellings(t, &ok) { - out.push( - json!({ "layer": id.0, "start": char_at(t, m.start), "end": char_at(t, m.end), "word": m.word, "suggestions": dict.suggest(&m.word, max) }), - ); + let found = dict.misspellings(t, &ok); + // Convert every start/end byte offset to a character offset in one pass over the + // text, instead of rescanning the prefix with char_at twice per misspelling. + let mut offsets = Vec::with_capacity(found.len() * 2); + for m in &found { + offsets.push(m.start); + offsets.push(m.end); + } + let chars = char_offsets(t, &offsets); + for (i, m) in found.iter().enumerate() { + out.push(json!({ "layer": id.0, "start": chars[i * 2], "end": chars[i * 2 + 1], "word": m.word, "suggestions": dict.suggest(&m.word, max) })); } } Ok(json!({ "misspellings": out, "count": out.len() })) @@ -263,6 +292,17 @@ pub fn specs() -> Vec { mod tests { use super::*; + #[test] + fn char_offsets_matches_per_offset_counting() { + // Multibyte text so byte offsets and char offsets differ. The batch result must + // equal counting chars in each prefix individually, including the end-of-text offset. + let text = "café naïve ☃ résumé"; + let targets: Vec = text.char_indices().map(|(b, _)| b).chain(std::iter::once(text.len())).collect(); + let got = char_offsets(text, &targets); + let want: Vec = targets.iter().map(|&b| text[..b.min(text.len())].chars().count()).collect(); + assert_eq!(got, want); + } + fn session(text: &str) -> (Session, u64) { let mut s = Session::new(); s.execute("file.new", json!({"width": 300, "height": 100, "background": "white"})).unwrap(); From 19e8c0d7cd14ae14b2f3b97b61597263e63e9ae0 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 22:00:01 -0400 Subject: [PATCH 25/34] security(codecs): stop quadratic rescans in XMP value scanning xmp_values advanced its cursor only past the matched key, so the value-closing search (find(close)) rescanned the remaining packet for every property-like substring. Repeating text such as ' tiff:ResolutionUnit>' inside the packet produced quadratic CPU work even when no real resolution property existed. Advance the cursor past each located value; a missing close delimiter means the value is unterminated, so no later value can parse and the scan stops. Valid multi-value extraction is unchanged. Finding: 6ce55e33-27f0-4fb9-a22f-737382e1d328 (CWE-407) --- crates/codecs/src/resolution.rs | 8 +++++++- crates/codecs/tests/resolution.rs | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/codecs/src/resolution.rs b/crates/codecs/src/resolution.rs index cb86001ac9..a787cacbc3 100644 --- a/crates/codecs/src/resolution.rs +++ b/crates/codecs/src/resolution.rs @@ -429,8 +429,14 @@ fn xmp_values(xmp: &str, key: &str) -> Vec<(usize, usize)> { continue; }; let start = xmp.len() - value.len(); - let Some(len) = value.find(close) else { continue }; + // Advance past this value before searching for the next key. Without this, `from` + // only moved past the key text, so property-like text repeated inside an unrelated + // attribute re-ran this full-suffix `find(close)` for every occurrence, which is + // quadratic in the packet size. A missing close means the value is unterminated, so + // no later value can parse either; stop scanning in that case. + let Some(len) = value.find(close) else { break }; out.push((start, start + len)); + from = start + len + close.len_utf8(); } out } diff --git a/crates/codecs/tests/resolution.rs b/crates/codecs/tests/resolution.rs index 15ddc95749..28c8203c96 100644 --- a/crates/codecs/tests/resolution.rs +++ b/crates/codecs/tests/resolution.rs @@ -284,6 +284,17 @@ fn truncated_metadata_never_panics_and_falls_back() { assert_eq!(xmp_resolution(x), None, "{x}"); let _ = xmp_with_resolution(x, (1.0, 1.0)); } + + // Property-like text repeated inside the packet must not trigger a full-suffix rescan + // per occurrence (it used to be quadratic). With the cursor advancing past each located + // value, this completes well under a second even with many decoys; a timeout here would + // indicate the quadratic scan returned. A clean packet still extracts correctly. + let start = std::time::Instant::now(); + let decoys = " tiff:ResolutionUnit>".repeat(50_000); + let _ = xmp_resolution(&decoys); + assert!(start.elapsed() < std::time::Duration::from_secs(2), "xmp scan took too long: {:?}", start.elapsed()); + let clean = "300/1300/12"; + near(xmp_resolution(clean), 300.0); } proptest! { From 3495a4b22517973edc23012fab0b0d6a888a84cd Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:57:49 -0400 Subject: [PATCH 26/34] security(codecs): dedup Cryptomatte keys and cache stream lists Cryptomatte metadata enumeration had two file-driven amplifiers: - cryptomatte_keys() tested key membership with iter().any(), O(n^2) in the number of distinct cryptomatte// attribute groups; - layers() called layer_streams() for every key, rescanning all channels and allocating a fresh owned stream list even when many keys name the same layer. Use a HashSet for key membership (keeping file order) and cache layer_streams results by resolved layer name within each header, so repeated keys reuse the discovered streams. Findings: - 120f4d51-b981-4fcc-abec-26f45f3efd29 (CWE-407, quadratic key membership) - dae6e76b-a491-4089-892f-4cf34b911f36 (CWE-400, repeated per-key stream scans) --- crates/codecs/src/codecs/exr_cryptomatte.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/codecs/src/codecs/exr_cryptomatte.rs b/crates/codecs/src/codecs/exr_cryptomatte.rs index 91a5ede22d..370c7c7e56 100644 --- a/crates/codecs/src/codecs/exr_cryptomatte.rs +++ b/crates/codecs/src/codecs/exr_cryptomatte.rs @@ -156,9 +156,13 @@ pub(crate) fn layers(bytes: &[u8], limits: &Limits) -> Result> = std::collections::HashMap::new(); for key in cryptomatte_keys(&header.own_attributes.other) { let Some(name) = attribute(&header.own_attributes.other, &key, "name") else { continue }; - let channels = layer_streams(header, name.as_str()); + let channels = streams_for.entry(name.clone()).or_insert_with(|| layer_streams(header, name.as_str())).clone(); if channels.is_empty() { continue; } @@ -184,11 +188,15 @@ pub(crate) fn layers(bytes: &[u8], limits: &Limits) -> Result/` key of a header, in file order, without duplicates. fn cryptomatte_keys(other: &std::collections::HashMap) -> Vec { let mut keys = Vec::new(); + // Track seen keys in a set: the attribute names are file-controlled, so membership with + // keys.iter().any() is O(n^2) in the number of distinct `cryptomatte//` + // groups. The Vec still records file order; the set only answers "seen before". + let mut seen = std::collections::HashSet::new(); for name in other.keys() { let full = name.to_string(); let Some(rest) = full.strip_prefix("cryptomatte/") else { continue }; let Some((key, _field)) = rest.split_once('/') else { continue }; - if !key.is_empty() && !keys.iter().any(|k| k == key) { + if !key.is_empty() && seen.insert(key.to_string()) { keys.push(key.to_string()); } } From dc6057d3f2cd425859cddeb485453d1b1498b107 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:56:22 -0400 Subject: [PATCH 27/34] security(engine): deduplicate ATN action names in near-linear time actions.load gave each imported action a unique name with a helper that restarted suffix numbering at 2 and linearly scanned all accepted names for every candidate. A file repeating one base name many times caused cubic comparison work during load. Replace it with a UniqueNames helper backed by a HashSet for membership and a per-base-name next-suffix cache, preserving the name, name 2, name 3 ... sequence and first-match behavior. Finding: 9c7a5fdb-131e-4687-8175-58d72007b0c5 (CWE-407) --- crates/engine/src/atn_cmds.rs | 37 ++++++++++++++++++++++------- crates/engine/src/atn_cmds/tests.rs | 12 ++++++++++ 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/atn_cmds.rs b/crates/engine/src/atn_cmds.rs index afba793027..3be9ee217a 100644 --- a/crates/engine/src/atn_cmds.rs +++ b/crates/engine/src/atn_cmds.rs @@ -3631,12 +3631,34 @@ pub fn export( // ------------------------------------------------------------------ commands -/// A name not used by any action in `taken`: `name`, `name 2`, `name 3`… -fn unique(name: &str, taken: &[String]) -> String { - if !taken.iter().any(|t| t == name) { - return name.to_string(); +/// Assigns names not already used, as `name`, `name 2`, `name 3`… A plain linear scan of the +/// taken list per candidate is cubic when an action file repeats one base name many times, so +/// membership uses a set and the next suffix to try is cached per base name. +struct UniqueNames { + taken: std::collections::HashSet, + next_suffix: std::collections::HashMap, +} + +impl UniqueNames { + fn new>(existing: I) -> Self { + UniqueNames { taken: existing.into_iter().collect(), next_suffix: std::collections::HashMap::new() } + } + + /// Reserves and returns a unique name derived from `name`, recording it as taken. + fn claim(&mut self, name: &str) -> String { + if self.taken.insert(name.to_string()) { + return name.to_string(); + } + let mut i = self.next_suffix.get(name).copied().unwrap_or(2); + loop { + let candidate = format!("{name} {i}"); + i += 1; + if self.taken.insert(candidate.clone()) { + self.next_suffix.insert(name.to_string(), i); + return candidate; + } + } } - (2..).map(|i| format!("{name} {i}")).find(|n| !taken.contains(n)).unwrap_or_else(|| name.to_string()) } fn load(s: &mut Session, p: &Value) -> Result { @@ -3664,19 +3686,18 @@ fn load(s: &mut Session, p: &Value) -> Result { if replace { s.actions.list.retain(|a| a.set.as_deref() != Some(imp.set.as_str())); } - let mut taken: Vec = s.actions.list.iter().map(|a| a.name.clone()).collect(); + let mut taken = UniqueNames::new(s.actions.list.iter().map(|a| a.name.clone())); let mut names = Vec::with_capacity(imp.actions.len()); // The file's name of each action that had to be renamed, and its new name. let mut renamed: std::collections::HashMap = std::collections::HashMap::new(); let mut steps = 0usize; let mut added = Vec::with_capacity(imp.actions.len()); for mut a in imp.actions { - let name = unique(&a.name, &taken); + let name = taken.claim(&a.name); if name != a.name { renamed.entry(a.name.clone()).or_insert_with(|| name.clone()); } a.name = name.clone(); - taken.push(name.clone()); steps = steps.saturating_add(a.steps.len()); names.push(name); added.push(a); diff --git a/crates/engine/src/atn_cmds/tests.rs b/crates/engine/src/atn_cmds/tests.rs index bbec1def43..e8595086ac 100644 --- a/crates/engine/src/atn_cmds/tests.rs +++ b/crates/engine/src/atn_cmds/tests.rs @@ -163,6 +163,18 @@ fn load_renames_each_play_target_once() { assert_eq!(s.actions.list[1].steps[0].1, json!({"action": "A"}), "B still plays the A it had"); } +#[test] +fn load_many_duplicate_names_get_sequential_suffixes() { + // Several actions share one base name. Deduplication must stay O(n) overall and produce + // the sequence A, A 2, A 3, A 4 (the next-suffix cache cannot skip or repeat a number). + let mut s = Session::new(); + let dupes: Vec = (0..4).map(|_| Action::new("A", vec![])).collect(); + let refs: Vec<&Action> = dupes.iter().collect(); + let (bytes, _) = export("File", &refs, &[], false).unwrap(); + let r = s.execute("actions.load", json!({"data": b64(&bytes)})).unwrap(); + assert_eq!(r["actions"], json!(["A", "A 2", "A 3", "A 4"])); +} + #[test] fn save_round_trips_through_photoshop_events() { let mut s = Session::new(); From f42d7ef41ac4cf5b9bde70052c6e157d14911eef Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:51:23 -0400 Subject: [PATCH 28/34] security(codecs): index FITS header keywords for O(1) lookup Header::get scanned every card linearly, and axis resolution looks up NAXIS1..NAXISn while find_image/data_len re-resolve the axes. Both the commentary-card count and the NAXIS count are file-controlled, so a crafted header multiplied them into quadratic header-processing work without a correspondingly large decoded image. Build a first-occurrence keyword index once in parse_header (skipping HIERARCH cards, matching the prior filter) and use it in get(). Finding: eb2121d8-9dee-4422-bd57-f78dce60a61b (CWE-407) --- crates/codecs/src/codecs/fits.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/codecs/src/codecs/fits.rs b/crates/codecs/src/codecs/fits.rs index 7e3f92ae67..214d968944 100644 --- a/crates/codecs/src/codecs/fits.rs +++ b/crates/codecs/src/codecs/fits.rs @@ -22,6 +22,8 @@ //! (strings longer than one card on `CONTINUE` cards). //! Tile-compressed files (fpack `ZIMAGE` tables) are recognised and refused. +use std::collections::HashMap; + use crate::Format; use crate::error::CodecError; use crate::fidelity::Plan; @@ -106,13 +108,19 @@ fn parse_card(raw: &[u8]) -> Card { struct Header { cards: Vec, + /// First-occurrence index of each non-HIERARCH keyword, so `get` is O(1). The card + /// count and the number of looked-up keywords (NAXIS1..NAXISn) are both file-controlled, + /// so a linear scan per lookup lets a crafted header multiply commentary cards by axis + /// count into quadratic work. + index: HashMap, /// Offset of the data unit. data_start: usize, } impl Header { fn get(&self, key: &str) -> Option<&Value> { - self.cards.iter().find(|c| !c.hierarch && c.key == key).and_then(|c| c.value.as_ref()) + let c = &self.cards[*self.index.get(key)?]; + c.value.as_ref() } fn int(&self, key: &str) -> Option { @@ -167,7 +175,15 @@ fn parse_header(b: &[u8], start: usize) -> Result { cards.push(card); } let data_start = pos.div_ceil(BLOCK).checked_mul(BLOCK).ok_or_else(|| err("header too long"))?; - Ok(Header { cards, data_start }) + // Index each non-HIERARCH keyword to its first occurrence, matching the previous + // `find(|c| !c.hierarch && c.key == key)` lookup while making `get` O(1). + let mut index = HashMap::with_capacity(cards.len()); + for (i, c) in cards.iter().enumerate() { + if !c.hierarch { + index.entry(c.key.clone()).or_insert(i); + } + } + Ok(Header { cards, index, data_start }) } /// The axis lengths (`NAXIS1`, `NAXIS2`, …). From e75790b8780fd77204ca7a255f34f854ec6e89af Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:50:14 -0400 Subject: [PATCH 29/34] security(io): index XCF parasite names for duplicate replacement parasites() implemented last-duplicate-wins by scanning all previously collected names with iter_mut().find() per record. Parasite records are file-controlled, so many distinct names caused O(n^2) CPU work before unrecognized metadata was discarded. Maintain a name-to-index map so replacement is O(1) while preserving insertion order and last-value-wins behavior. Finding: 6107f41e-9b17-4632-a9df-f88a74a796a8 (CWE-407) --- crates/io/src/xcf.rs | 15 ++++++++++++--- crates/io/src/xcf/tests.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/crates/io/src/xcf.rs b/crates/io/src/xcf.rs index 4bb62cacbb..d5cda4aa4d 100644 --- a/crates/io/src/xcf.rs +++ b/crates/io/src/xcf.rs @@ -17,6 +17,7 @@ #[cfg(test)] mod tests; +use std::collections::HashMap; use std::io::Read; use std::sync::Arc; @@ -294,13 +295,21 @@ fn be_f32(b: &[u8]) -> Option { fn parasites(data: &[u8]) -> Vec<(String, Vec)> { let mut c = Cursor { b: data, pos: 0, wide: false }; let mut out: Vec<(String, Vec)> = Vec::new(); + // Map each name to its output slot so duplicate replacement is O(1). The parasite + // records are file-controlled, so scanning `out` with iter_mut().find() per record is + // O(n^2) and lets a crafted payload with many distinct names stall import. The map + // preserves insertion order and last-value-wins behavior. + let mut index: HashMap = HashMap::new(); while c.pos < data.len() { let Ok(name) = c.string() else { break }; let (Ok(_flags), Ok(len)) = (c.u32(), c.u32()) else { break }; let Ok(payload) = c.take(len as usize) else { break }; - match out.iter_mut().find(|(n, _)| *n == name) { - Some(slot) => slot.1 = payload.to_vec(), - None => out.push((name, payload.to_vec())), + match index.get(&name) { + Some(&i) => out[i].1 = payload.to_vec(), + None => { + index.insert(name.clone(), out.len()); + out.push((name, payload.to_vec())); + } } } out diff --git a/crates/io/src/xcf/tests.rs b/crates/io/src/xcf/tests.rs index ee96f16509..400887f92b 100644 --- a/crates/io/src/xcf/tests.rs +++ b/crates/io/src/xcf/tests.rs @@ -702,3 +702,33 @@ fn a_path_failing_below_the_top_level_lands_at_the_top_level() { assert_eq!(children, ["A"], "the valid sibling placed after the stray one still lands in the group"); assert!(r.warnings.iter().any(|w| w.contains("Stray") && w.contains("item path")), "{:?}", r.warnings); } + +/// A parasite name field: u32 `len+1`, then the bytes and a trailing NUL. +fn parasite_name(out: &mut Vec, name: &str) { + out.extend_from_slice(&((name.len() as u32) + 1).to_be_bytes()); + out.extend_from_slice(name.as_bytes()); + out.push(0); +} + +/// One PROP_PARASITES record: name, u32 flags, u32 len, then the payload. +fn parasite_record(out: &mut Vec, name: &str, payload: &[u8]) { + parasite_name(out, name); + out.extend_from_slice(&0u32.to_be_bytes()); + out.extend_from_slice(&(payload.len() as u32).to_be_bytes()); + out.extend_from_slice(payload); +} + +#[test] +fn duplicate_parasites_keep_order_and_last_value() { + // Duplicate names must resolve to the last payload while keeping first-seen order. + // This is the behavior the name->index map must preserve versus the old linear scan. + let mut data = Vec::new(); + parasite_record(&mut data, "gimp-comment", b"first"); + parasite_record(&mut data, "gimp-image-grid", b"grid"); + parasite_record(&mut data, "gimp-comment", b"second"); + let out = parasites(&data); + assert_eq!(out.len(), 2, "duplicate collapses to one slot"); + assert_eq!(out[0].0, "gimp-comment"); + assert_eq!(out[0].1, b"second", "last value wins"); + assert_eq!(out[1].0, "gimp-image-grid", "insertion order preserved"); +} From 7c78c951e44f6f92869679522a80a57be4c22d74 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:47:53 -0400 Subject: [PATCH 30/34] security(io): index comp IDs in PSD layer-comp import decode_comps matched each nonzero layer-setting comp ID against the comp vector with iter_mut().find(), an O(n*m) scan over two file-controlled collections. A crafted PSD could cause quadratic CPU work during import. Build a comp-ID-to-first-index map once and use it for setting lookups. or_insert preserves first-match behavior for duplicate comp IDs; comp 0 (Last Document State) is still handled separately. Finding: 47567ac0-a75f-4557-b127-ffc855e5bd8d (CWE-407) --- crates/io/src/comps_map.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/io/src/comps_map.rs b/crates/io/src/comps_map.rs index 4cade56e2e..3348061b34 100644 --- a/crates/io/src/comps_map.rs +++ b/crates/io/src/comps_map.rs @@ -12,6 +12,8 @@ //! Both are re-emitted verbatim while they still decode to the document's state, and are //! regenerated otherwise (comp appearance is not written to PSD; `.pcraft` keeps it). +use std::collections::HashMap; + use photocraft_doc::comps::layer_position; use photocraft_doc::{Artboard, ArtboardBackground, CompLayerState, Document, Guides, Layer, LayerComp}; use photocraft_geom::Rect; @@ -203,6 +205,14 @@ pub(crate) fn decode_comps(resource: Option<&[u8]>, doc: &Document) -> (Vec = HashMap::with_capacity(comps.len()); + for (i, c) in comps.iter().enumerate() { + id_to_index.entry(c.id).or_insert(i); + } let mut last = LayerComp { id: 0, name: "Last Document State".into(), @@ -239,7 +249,7 @@ pub(crate) fn decode_comps(resource: Option<&[u8]>, doc: &Document) -> (Vec None, }; for id in &s.comps { - let target = if *id == 0 { Some(&mut last) } else { comps.iter_mut().find(|c| c.id == *id) }; + let target = if *id == 0 { Some(&mut last) } else { id_to_index.get(id).map(|&i| &mut comps[i]) }; let Some(c) = target else { continue }; let visible = if *id != 0 && c.apply_visibility { Some(resolved) } else { s.visible }; if visible.is_none() && s.offset.is_none() { From 81c694a3f99da438231aff292232a75741db74f9 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:13:12 -0400 Subject: [PATCH 31/34] security: bound RAW linked-directory metadata reads --- crates/raw/src/preview.rs | 13 +++++++------ crates/raw/src/tiff.rs | 25 ++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/raw/src/preview.rs b/crates/raw/src/preview.rs index d5dd013310..9f63173bf6 100644 --- a/crates/raw/src/preview.rs +++ b/crates/raw/src/preview.rs @@ -74,12 +74,13 @@ pub fn embedded_previews(bytes: &[u8]) -> Vec> { if let (Some(off), Some(len)) = (t.tag_uint(&ifd, tag::JPEG_INTERCHANGE_FORMAT), t.tag_uint(&ifd, tag::JPEG_INTERCHANGE_FORMAT_LENGTH)) { found.extend(candidate(bytes, off as usize, len as usize)); } - if matches!(t.tag_uint(&ifd, tag::COMPRESSION), Some(6 | 7)) { - let offs = t.tag_uints(&ifd, tag::STRIP_OFFSETS); - let lens = t.tag_uints(&ifd, tag::STRIP_BYTE_COUNTS); - if let ([off], [len]) = (offs.as_slice(), lens.as_slice()) { - found.extend(candidate(bytes, *off as usize, *len as usize)); - } + if matches!(t.tag_uint(&ifd, tag::COMPRESSION), Some(6 | 7)) + && let (Some(offsets), Some(lengths)) = (ifd.get(tag::STRIP_OFFSETS), ifd.get(tag::STRIP_BYTE_COUNTS)) + && offsets.count == 1 + && lengths.count == 1 + && let (Some(off), Some(len)) = (t.uint(offsets, 0), t.uint(lengths, 0)) + { + found.extend(candidate(bytes, off as usize, len as usize)); } // Panasonic RW2 JpgFromRaw (0x002E) holds a complete JPEG. if let Some(e) = ifd.get(0x002E).filter(|e| e.typ == 7) diff --git a/crates/raw/src/tiff.rs b/crates/raw/src/tiff.rs index cfcadee730..0fa11685f7 100644 --- a/crates/raw/src/tiff.rs +++ b/crates/raw/src/tiff.rs @@ -213,9 +213,11 @@ impl<'a> Tiff<'a> { } for t in [tag::SUB_IFDS, tag::EXIF_IFD] { if let Some(e) = ifd.get(t) { - // Reverse so the first SubIFD is visited first. - let mut v = self.uints(e); - v.truncate(16); + // Linked-directory traversal only needs the first sixteen offsets. Read + // those values directly instead of expanding the complete file-controlled + // array and truncating it afterward. + let n = (e.count as usize).min(16); + let v = (0..n).map(|i| self.uint(e, i)).collect::>>().unwrap_or_default(); stack.extend(v.into_iter().rev().map(|o| o as usize)); } } @@ -450,3 +452,20 @@ mod tests { assert!(out.contains("output truncated") || out.len() < 1 << 20); } } + +#[test] +fn linked_directory_values_are_read_only_as_needed() { + // The traversal limit is independent of the TIFF entry count: callers should + // never need a complete attacker-controlled offset array to visit SubIFDs. + let mut b = b"II*\0\x08\0\0\0".to_vec(); + b.extend_from_slice(&1u16.to_le_bytes()); + b.extend_from_slice(&330u16.to_le_bytes()); + b.extend_from_slice(&4u16.to_le_bytes()); + b.extend_from_slice(&(1_000_000u32).to_le_bytes()); + b.extend_from_slice(&26u32.to_le_bytes()); + b.extend_from_slice(&0u32.to_le_bytes()); + b.extend_from_slice(&0u32.to_le_bytes()); + b.resize(26 + 16 * 4, 0); + let t = Tiff::new(&b).unwrap(); + assert!(t.all_ifds().len() <= 17); +} From 5338f1de46aa58e4bb119302e126eed07250f844 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 21 Oct 2026 21:13:11 -0400 Subject: [PATCH 32/34] security: budget nested action playback and preserve batch auth --- crates/engine/src/actions_cmds.rs | 34 +++++++++++++++++++++++++++++++ crates/engine/src/file_cmds.rs | 4 +++- crates/ui-egui/src/actions.rs | 4 ++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/actions_cmds.rs b/crates/engine/src/actions_cmds.rs index 9babc5a50a..8820439a0d 100644 --- a/crates/engine/src/actions_cmds.rs +++ b/crates/engine/src/actions_cmds.rs @@ -81,6 +81,8 @@ pub struct ActionsFile { pub actions: Vec, } +pub const MAX_PLAYBACK_STEPS: u64 = 100_000; + /// Actions list and the recording / playback flags. Only `list` is persisted. #[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] #[serde(default)] @@ -95,6 +97,9 @@ pub struct ActionState { /// Active action names, for cycle detection in engine and desktop playback. #[serde(skip)] pub playback_stack: Vec, + /// Number of steps executed by the current top-level playback tree. + #[serde(skip)] + pub playback_steps: u64, /// Bumped whenever `list` changes, so the preset store can skip unchanged writes. #[serde(skip)] pub rev: u64, @@ -143,6 +148,15 @@ pub fn save_target_closed(s: &Session, id: &str, target: Option bool { + if s.actions.playback_steps >= MAX_PLAYBACK_STEPS { + false + } else { + s.actions.playback_steps += 1; + true + } +} /// View-only menu commands recorded by the desktop shell. Headless playback reports /// them as unsupported instead of silently dropping them; Batch and droplets skip them. pub fn shell_view_command(id: &str) -> bool { @@ -348,6 +362,10 @@ fn run_recorded(s: &mut Session, action: &Action, from: usize) -> (u64, Option (u64, Option Result> { flush_recording(s)?; + if s.actions.playing == 0 { + s.actions.playback_steps = 0; + } let recording = s.actions.recording.take(); s.actions.playing = s.actions.playing.saturating_add(1); s.actions.playback_stack.push(action.name.clone()); @@ -1583,3 +1604,16 @@ mod tests { assert!(s.actions.list[0].color.is_empty(), "rejected colours change nothing"); } } + +#[cfg(test)] +mod security_tests { + use super::*; + + #[test] + fn playback_budget_is_shared_by_nested_calls() { + let mut session = Session::new(); + session.actions.playback_steps = MAX_PLAYBACK_STEPS - 1; + assert!(consume_playback_step(&mut session)); + assert!(!consume_playback_step(&mut session)); + } +} diff --git a/crates/engine/src/file_cmds.rs b/crates/engine/src/file_cmds.rs index 0bb2b78928..1833d0a616 100644 --- a/crates/engine/src/file_cmds.rs +++ b/crates/engine/src/file_cmds.rs @@ -1259,7 +1259,9 @@ fn batch(s: &mut Session, p: &Value) -> Result { let actions = crate::actions_cmds::ActionState { list, playing: s.actions.playing, playback_stack: s.actions.playback_stack.clone(), ..Default::default() }; let names = batch_naming::plan(&inputs, &output, &format, p)?; let save = SaveOpts::from_params(s, p, cmd)?; - let run = |scratch: &mut Session| { + let authorize = s.authorize; + let run = move |scratch: &mut Session| { + scratch.authorize = authorize; scratch.actions = actions.clone(); for (id, params) in &steps { let r = scratch.execute(id, params.clone())?; diff --git a/crates/ui-egui/src/actions.rs b/crates/ui-egui/src/actions.rs index 8a7ce9bf39..a9fbc56add 100644 --- a/crates/ui-egui/src/actions.rs +++ b/crates/ui-egui/src/actions.rs @@ -101,6 +101,10 @@ pub(crate) fn play(app: &mut PhotocraftApp, params: &Value) -> Result Date: Wed, 21 Oct 2026 21:13:12 -0400 Subject: [PATCH 33/34] security: encode printer names in IPP request paths --- crates/ipp/src/lib.rs | 17 ++++++++++++++--- crates/ipp/src/tests.rs | 6 ++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/crates/ipp/src/lib.rs b/crates/ipp/src/lib.rs index 9f1209df45..0f8f303ee0 100644 --- a/crates/ipp/src/lib.rs +++ b/crates/ipp/src/lib.rs @@ -480,8 +480,19 @@ fn dechunk(mut b: &[u8]) -> Result> { } /// Where `/path` lands on the server: `/printers/` or `/classes/`. -fn queue_path(name: &str, is_class: bool) -> String { - format!("/{}/{name}", if is_class { "classes" } else { "printers" }) +fn queue_path(name: &str, is_class: bool) -> Result { + if name.is_empty() || name.bytes().any(|b| b < 0x20 || b == 0x7f) { + return Err(Error::Protocol("printer name contains an invalid control character".into())); + } + let mut encoded = String::with_capacity(name.len()); + for &b in name.as_bytes() { + if b.is_ascii_alphanumeric() || matches!(b, b'-' | b'.' | b'_' | b'~') { + encoded.push(char::from(b)); + } else { + encoded.push_str(&format!("%{b:02X}")); + } + } + Ok(format!("/{}/{encoded}", if is_class { "classes" } else { "printers" })) } /// The printers the server knows (CUPS-Get-Printers), then its classes (CUPS-Get-Classes, which @@ -568,7 +579,7 @@ pub struct Job<'a> { /// Submits `job` (Print-Job) and returns the job id. pub fn print_job(endpoint: &Endpoint, job: &Job<'_>) -> Result { let mut r = Request::new(OP_PRINT_JOB, 3); - let path = queue_path(job.printer, job.is_class); + let path = queue_path(job.printer, job.is_class)?; r.attr(TAG_URI, "printer-uri", format!("ipp://{}{path}", endpoint.host()).as_bytes()); r.attr(TAG_NAME, "requesting-user-name", job.user.as_bytes()); r.attr(TAG_NAME, "job-name", job.title.as_bytes()); diff --git a/crates/ipp/src/tests.rs b/crates/ipp/src/tests.rs index ff3ccb6a49..ab236d75ee 100644 --- a/crates/ipp/src/tests.rs +++ b/crates/ipp/src/tests.rs @@ -393,3 +393,9 @@ fn an_impossible_chunk_size_is_a_protocol_error_not_a_panic() { } } } + +#[test] +fn printer_names_are_encoded_as_one_request_target_segment() { + assert_eq!(queue_path("office printer/1", false).unwrap(), "/printers/office%20printer%2F1"); + assert!(queue_path("printer\r\nX-Injected: yes", false).is_err()); +} From 1104b6f2111a4c66adfe6b051d710712124a5bbc Mon Sep 17 00:00:00 2001 From: zuujiscom Date: Thu, 22 Oct 2026 00:26:35 -0500 Subject: [PATCH 34/34] ci(freebsd): run the photocraft-gpu tests one at a time The nightly FreeBSD run fails because the timing test binary crashes with SIGSEGV in the VM's software Vulkan driver when its tests set up devices in parallel. Run with --test-threads=1 it passed 3 of 3 times on GitHub's x86_64 and aarch64 VMs and on a native FreeBSD 15.1 aarch64 VM, where all 121 photocraft-gpu tests pass. Exclude photocraft-gpu from the workspace test run and test it serially, as ui-egui's integration tests already are. --- .github/workflows/freebsd.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml index 970c6577a4..56d1ad0831 100644 --- a/.github/workflows/freebsd.yml +++ b/.github/workflows/freebsd.yml @@ -32,5 +32,7 @@ jobs: with: craft-fonts-ref: abb83316d96aa59c1cf64784289e378fe9fa5695 cargo-build-extra-args: "--features heif" - custom-test-command: "CARGO_PROFILE_DEV_DEBUG=0 cargo test --workspace --locked --features heif --exclude photocraft-ui-egui && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --lib && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --doc && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --test '*' -- --test-threads=1" + # The VM's software Vulkan driver crashes (SIGSEGV) when several tests set up a device at + # once, so photocraft-gpu's tests run one at a time, like ui-egui's integration tests. + custom-test-command: "CARGO_PROFILE_DEV_DEBUG=0 cargo test --workspace --locked --features heif --exclude photocraft-ui-egui --exclude photocraft-gpu && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-gpu --locked -- --test-threads=1 && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --lib && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --doc && CARGO_PROFILE_DEV_DEBUG=0 cargo test -p photocraft-ui-egui --locked --test '*' -- --test-threads=1" run-package-check: true