mirror of
https://github.com/storytold/vectorcraft.git
synced 2026-11-01 13:47:21 -06:00
SVG import: a feConvolveMatrix order that overflows usvg's multiply is refused
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 <noreply@z.ai>
This commit is contained in:
committed by
bflatastic
co-authored by
GLM-5.3-Flash
parent
1679318034
commit
c7dc443ee1
@@ -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());
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
Reference in New Issue
Block a user