From c7dc443ee14a53f2ebdc94aa98fae780ca7a52a0 Mon Sep 17 00:00:00 2001 From: SusieeTheFemboy <96988330+SusieeTheLinuxUser@users.noreply.github.com> Date: Sat, 24 Oct 2026 03:30:54 +0200 Subject: [PATCH 1/2] SVG import: a feConvolveMatrix order that overflows usvg's multiply is refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit usvg 0.48 casts the filter's order attribute through i32, so a value like 1066570220609 saturates and the u32 order_x * order_y multiply overflows. Only debug and overflow-checks builds panic on that; release builds already import the file — the multiply wraps, and VectorCraft drops the filter as an unsupported primitive with a warning, so nothing reaches the document, render or export (observed on main in a --release build: both repro files import without error; with a one-entry kernelMatrix the wrapped product 2147483647² mod 2³² = 1 even passes usvg's length check and leaves a filter claiming a 2147483647×2147483647 kernel, still dropped downstream). The import now parses order the way usvg does (svgtypes numbers, the same as-i32 cast with its saturation, the first side defaulting the second, both positive or both back to the 3×3 default) and refuses exactly the one case: when the product of the two sides overflows u32 (checked_mul). Any order usvg can multiply keeps importing as before, however large on its own, and a missing, unparseable or non-positive order still falls back to 3. Regression tests cover the overflow repros, the release wrap case, large but fitting orders (100, 80 80, 65535 65535, 2147483647 1), the pass-through shapes, and sane kernels, in debug and in release. Co-Authored-By: GLM-5.3-Flash --- crates/svg/src/import.rs | 41 ++++++++++++++ crates/svg/tests/convolve_overflow.rs | 82 +++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 crates/svg/tests/convolve_overflow.rs diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index ff2e63c35..18f4f088d 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -62,6 +62,44 @@ fn parse(svg: &str) -> Result, SvgError> { .map_err(|e| SvgError::Parse(e.to_string())) } +const SVG_NS: &str = "http://www.w3.org/2000/svg"; + +/// Refuse the one `feConvolveMatrix` usvg cannot handle: an `order` whose two sides overflow the +/// `order_x * order_y` u32 multiply its parser makes (`usvg-0.48.1/src/parser/filter.rs:673`, and +/// `tree/filter.rs:458` through `ConvolveMatrixData::new`). usvg parses `order` through f64 and +/// `as i32`, so a large finite value like `1066570220609` saturates to i32::MAX — positive, so it +/// passes the positivity guard — and the multiply overflows: a debug or overflow-checks build +/// panics while opening the file. A release build wraps instead, and with a one-entry +/// `kernelMatrix` the wrapped product passes usvg's length check, leaving a filter that claims a +/// 2147483647×2147483647 kernel (which VectorCraft then drops as an unsupported filter primitive, +/// so nothing reaches the document). Any order usvg can multiply — however large — keeps +/// importing exactly as before: a missing, unparseable or non-positive `order` falls back to 3, +/// and a `kernelMatrix` that fails to parse or match the order leaves the filter empty, which +/// usvg drops. The attribute read here is the one usvg reads: unprefixed (XML keeps unprefixed +/// attributes outside any namespace), on an element in the SVG namespace, with entities resolved +/// by the parse above. +fn convolve_matrices_ok(xml: &roxmltree::Document) -> Result<(), SvgError> { + for fe in xml.descendants() { + if !fe.has_tag_name((SVG_NS, "feConvolveMatrix")) { + continue; + } + if let Some(value) = fe.attribute("order") { + // The parse usvg does: svgtypes numbers, `as i32` (saturation included), the first + // side defaulting the second, and both positive or both back to the 3×3 default. + let side = |n: Option>, default: i32| n.and_then(|v| v.ok()).map(|v| v as i32).unwrap_or(default); + let mut numbers = svgtypes::NumberListParser::from(value); + let x = side(numbers.next(), 3); + let y = side(numbers.next(), x); + if x > 0 && y > 0 && (x as u32).checked_mul(y as u32).is_none() { + return Err(SvgError::Parse(format!( + "a feConvolveMatrix filter's order is {x}×{y}, which overflows the kernel-size multiply usvg makes" + ))); + } + } + } + Ok(()) +} + /// The document SVG text `svg` holds, read with `opts`, and the warnings about what it couldn't keep. pub(crate) fn import(svg: &str, opts: &ImportOptions, max_nodes: usize) -> Result<(Document, Vec), SvgError> { // Before anything reads the tree: the XML reader, usvg, and what is made of its tree, all by @@ -81,6 +119,9 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions, max_nodes: usize) -> Resul Cow::Borrowed(_) => xml, Cow::Owned(edited) => parse(edited)?, }; + // Before usvg sees the XML: its parser overflows a hostile feConvolveMatrix's order + // (`convolve_matrices_ok` below), and there is no panic guard to fall back on. + convolve_matrices_ok(&xml)?; let svg = svg.as_ref(); let units = RootUnits::of(xml.root_element()); let root_name = root_layer_name(xml.root_element()); diff --git a/crates/svg/tests/convolve_overflow.rs b/crates/svg/tests/convolve_overflow.rs new file mode 100644 index 000000000..4c080e194 --- /dev/null +++ b/crates/svg/tests/convolve_overflow.rs @@ -0,0 +1,82 @@ +//! A `` whose `order` overflows usvg's `order_x * order_y` u32 multiply must be +//! refused before usvg sees it: usvg 0.48 parses the attribute through f64 and casts it with +//! `as i32`, so a large value like `1066570220609` saturates to i32::MAX and the multiply +//! overflows (a debug or overflow-checks build panics). A release build wraps instead, and with a +//! one-entry `kernelMatrix` the wrapped product passes usvg's length check, leaving a filter that +//! claims a 2147483647×2147483647 kernel. Everything else is usvg's pass-through and must keep +//! importing, however large the order is on its own. +// Integration tests: unwrapping and panicking on failure is fine here, unlike in shipped code (AGENTS.md › Robustness). +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use vectorcraft_svg::import; + +/// An overflowing `order` with a `kernelMatrix` that parses (usvg `parser/filter.rs`). +#[test] +fn convolve_matrix_overflowing_order_with_kernel_is_refused() { + let svg = r#""#; + let r = import(svg); + assert!(r.is_err(), "an order that overflows the multiply must be refused, not imported"); +} + +/// The release-wrap case: a one-entry `kernelMatrix`, whose wrapped `order_x * order_y` product +/// (2147483647² mod 2³² = 1) passes usvg's length check. +#[test] +fn convolve_matrix_overflowing_order_with_wrapping_kernel_is_refused() { + let svg = r#""#; + let r = import(svg); + assert!(r.is_err(), "an overflowing order must be refused even when a kernel would wrap past the length check"); +} + +/// An `order` whose second side fails to parse defaults it to the first (svgtypes rejects +/// out-of-range numbers like `-1e999`), so this is huge on both sides and refused too. +#[test] +fn convolve_matrix_overflowing_order_with_defaulted_second_side_is_refused() { + let svg = r#""#; + let r = import(svg); + assert!(r.is_err(), "an order that overflows the multiply must be refused, not imported"); +} + +/// An order usvg can multiply keeps importing, however large on its own: only the overflowing +/// product is refused. +#[test] +fn convolve_matrix_large_but_fitting_order_still_imports() { + for order in ["100", "80 80", "65535 65535", "2147483647 1"] { + let svg = format!( + r#""# + ); + assert!(import(&svg).is_ok(), "order={order} fits the u32 multiply and must keep importing"); + } +} + +/// usvg falls back to 3 when an order doesn't parse or isn't positive: those keep importing. +#[test] +fn convolve_matrix_nonpositive_or_unparseable_order_still_imports() { + let kernel = r#"kernelMatrix="0 0 0 0 1 0 0 0 0""#; + for order in ["0", "-3", "-1e999", "nan"] { + let svg = format!( + r#""# + ); + assert!(import(&svg).is_ok(), "order={order} is usvg's pass-through and must keep importing"); + } +} + +/// A `kernelMatrix` that doesn't parse or doesn't match the order leaves the filter empty, which +/// usvg drops: the file keeps importing (the filter is drawn without, as a warning). +#[test] +fn convolve_matrix_unparseable_or_mismatched_kernel_still_imports() { + let mismatched = r#""#; + assert!(import(mismatched).is_ok(), "a kernel of the wrong length is usvg's pass-through"); + let unparseable = r#""#; + assert!(import(unparseable).is_ok(), "an unparseable kernel is usvg's pass-through"); +} + +/// Sane kernels keep importing: 3×3, 2×2, and a defaulted order. +#[test] +fn sane_convolve_matrices_still_import() { + let three = r#""#; + assert!(import(three).is_ok(), "an ordinary filter must still import"); + let two = r#""#; + assert!(import(two).is_ok(), "a small kernel must still import"); + let defaulted = r#""#; + assert!(import(defaulted).is_ok(), "a filter without order must still import"); +} From 2e7b2b7f96478842cacdd4272261c70b4b6f5b5e Mon Sep 17 00:00:00 2001 From: SusieeTheFemboy <96988330+SusieeTheLinuxUser@users.noreply.github.com> Date: Sat, 31 Oct 2026 04:12:24 +0100 Subject: [PATCH 2/2] SVG import: an embedded SVG image gets the feConvolveMatrix order check too Files::svg parsed nested SVG images with usvg without the check the top-level import makes, so an overflowing order inside an still reached usvg's multiply. Co-Authored-By: Claude Opus 5.5 --- crates/svg/src/import/files.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/crates/svg/src/import/files.rs b/crates/svg/src/import/files.rs index 722d043d4..b6d488c9e 100644 --- a/crates/svg/src/import/files.rs +++ b/crates/svg/src/import/files.rs @@ -287,8 +287,10 @@ impl Files { if depth::xml_deeper_than(text, left) { return leave_out(&self.too_deep, true); } - let height = depth::height(&super::parse(text).ok()?, left); - let Some(height) = height else { return leave_out(&self.too_deep, true) }; + let xml = super::parse(text).ok()?; + let Some(height) = depth::height(&xml, left) else { return leave_out(&self.too_deep, true) }; + // usvg's parser would overflow on it, as on the SVG holding it (`import`). + super::convolve_matrices_ok(&xml).ok()?; self.note_text(text); self.depth.store(used + height, Ordering::Relaxed); let tree = usvg::Tree::from_data_nested(text.as_bytes(), opts); @@ -462,6 +464,22 @@ mod tests { assert_eq!(read(MAX_ELEMENT_DEPTH, 0), (false, 1)); } + /// An SVG image with a `feConvolveMatrix` order usvg's multiply overflows is left out before + /// usvg reads it, as the SVG holding it would be refused. + #[test] + fn svg_images_with_an_overflowing_convolve_order_are_left_out() { + let read = |order: &str| { + let svg = format!( + r#""# + ); + let files = Files::default(); + let resolver = files.resolver(1); + matches!((resolver.resolve_data)("image/svg+xml", Arc::new(svg.into_bytes()), &usvg::Options::default()), Some(ImageKind::SVG(_))) + }; + assert!(!read("2147483647")); + assert!(read("1")); + } + #[test] fn the_placeholder_is_a_png() { let png = marker().unwrap();