Merge pull request #4785 from SusieeTheLinuxUser/fix/nevercrash-svg-convolve-overflow

SVG import: a feConvolveMatrix order that overflows usvg's multiply is refused

crates/svg/src/import.rs

@@ -62,6 +62,44 @@ fn parse(svg: &str) -> Result<roxmltree::Document<'_>, 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<Result<f64, svgtypes::Error>>, 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<String>), 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());

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#"<svg xmlns="http://www.w3.org/2000/svg" width="4" height="4"><filter id="f"><feConvolveMatrix order="{order}" kernelMatrix="1"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#
+            );
+            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();

crates/svg/tests/convolve_overflow.rs

@@ -0,0 +1,82 @@
+//! A `<feConvolveMatrix>` 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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix kernelMatrix="1 2 3 4" order="1066570220609 17"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="2147483647" kernelMatrix="1"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix kernelMatrix="-60173 1e-999 1e999 0" order="946183596582 -1e999" divisor="-1e999"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="{order}"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#
+        );
+        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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="{order}" {kernel}/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#
+        );
+        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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="3" kernelMatrix="0 0 0 0 1 0 0 0"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    assert!(import(mismatched).is_ok(), "a kernel of the wrong length is usvg's pass-through");
+    let unparseable = r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="3" kernelMatrix="1e999 0 0 0 0 0 0 0 0"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    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#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="3" kernelMatrix="0 0 0 0 1 0 0 0 0"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    assert!(import(three).is_ok(), "an ordinary filter must still import");
+    let two = r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix order="2" kernelMatrix="0 1 1 0"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    assert!(import(two).is_ok(), "a small kernel must still import");
+    let defaulted = r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><filter id="f"><feConvolveMatrix kernelMatrix="0 0 0 0 1 0 0 0 0"/></filter><rect width="4" height="4" filter="url(#f)"/></svg>"#;
+    assert!(import(defaulted).is_ok(), "a filter without order must still import");
+}