mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-27 19:47:12 -06:00
svg: bound <use> expansion by an element budget and skip reference cycles (#1236)
Depth-limited <use> expansion still let a few kilobytes of branching references expand to 2^N elements. The parser now counts every element it converts, <use> copies included, against a budget of max(500 000, twice the file's own elements) and fails the import with Error::Xml once it is spent. Targets currently being expanded are tracked, so mutual references stop at the first repeat instead of nesting to the depth limit. Fixes #1236 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N9kwMcw2pYQ5fSjKa3ZPnh
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
549e9d8251
commit
79ebd01935
+23
-3
@@ -486,6 +486,12 @@ struct Parser<'a, 'i> {
|
||||
vw: f64,
|
||||
vh: f64,
|
||||
skipped: Vec<String>,
|
||||
/// Elements converted so far, `<use>` copies included, and how many may be (#1236).
|
||||
work: usize,
|
||||
budget: usize,
|
||||
/// The `<use>` targets being expanded, outermost first: a reference back into one is a
|
||||
/// cycle and is skipped.
|
||||
using: Vec<roxmltree::NodeId>,
|
||||
}
|
||||
|
||||
fn is_svg_ns(n: &roxmltree::Node) -> bool {
|
||||
@@ -748,9 +754,12 @@ impl<'a, 'i> Parser<'a, 'i> {
|
||||
|
||||
/// Convert an element (and its subtree) into nodes appended to `out`.
|
||||
fn node(&mut self, n: roxmltree::Node<'a, 'i>, parent: &Style, out: &mut Vec<Node>, depth: usize) {
|
||||
if depth > 64 || !is_svg_ns(&n) {
|
||||
if depth > 64 || !is_svg_ns(&n) || self.work > self.budget {
|
||||
return;
|
||||
}
|
||||
// Every element converted counts, each `<use>` copy again: a few references to groups
|
||||
// that reference each other several times can multiply into billions (#1236).
|
||||
self.work += 1;
|
||||
let tag = n.tag_name().name();
|
||||
match tag {
|
||||
"defs" | "title" | "desc" | "metadata" | "style" | "linearGradient" | "radialGradient" | "symbol" | "clipPath" | "mask" | "pattern" | "marker"
|
||||
@@ -816,9 +825,10 @@ impl<'a, 'i> Parser<'a, 'i> {
|
||||
}
|
||||
"use" => {
|
||||
let Some(target) = self.href(n) else { return };
|
||||
if target == n || n.ancestors().any(|a| a == target) {
|
||||
if target == n || n.ancestors().any(|a| a == target) || self.using.contains(&target.id()) {
|
||||
return;
|
||||
}
|
||||
self.using.push(target.id());
|
||||
let x = self.attr_len(n, "x", 'x', &st);
|
||||
let y = self.attr_len(n, "y", 'y', &st);
|
||||
transform *= Affine::translate((x, y));
|
||||
@@ -847,6 +857,7 @@ impl<'a, 'i> Parser<'a, 'i> {
|
||||
} else {
|
||||
self.node(target, &st, &mut children, depth + 1);
|
||||
}
|
||||
self.using.pop();
|
||||
out.push(Node::Group(Group {
|
||||
name: Self::element_name(n),
|
||||
transform,
|
||||
@@ -1048,6 +1059,9 @@ pub fn parse_transform(s: &str) -> Affine {
|
||||
/// 2 MiB thread's stack (aborting the app) at a few thousand levels; real documents stay far
|
||||
/// below this.
|
||||
const MAX_XML_DEPTH: usize = 256;
|
||||
/// The most elements an import converts, `<use>` copies included, unless the file itself has
|
||||
/// more than half as many (#1236).
|
||||
const MAX_EXPANDED_ELEMENTS: usize = 500_000;
|
||||
|
||||
/// The deepest element nesting of `text` (a quick scan that skips comments, CDATA, processing
|
||||
/// instructions, declarations and quoted attribute values).
|
||||
@@ -1121,7 +1135,9 @@ pub fn parse(bytes: &[u8]) -> Result<Doc, Error> {
|
||||
}
|
||||
}
|
||||
let vb = viewbox(root);
|
||||
let mut p = Parser { ids, rules, vw: vb.map_or(300.0, |v| v.1), vh: vb.map_or(150.0, |v| v.2), skipped: vec![] };
|
||||
// Room for generous reuse, and always for every element of the file itself.
|
||||
let budget = MAX_EXPANDED_ELEMENTS.max(root.descendants().filter(|n| n.is_element()).count().saturating_mul(2));
|
||||
let mut p = Parser { ids, rules, vw: vb.map_or(300.0, |v| v.1), vh: vb.map_or(150.0, |v| v.2), skipped: vec![], work: 0, budget, using: vec![] };
|
||||
let st = Style::default();
|
||||
let pct_or = |name: &str, axis: char, def: f64| -> f64 {
|
||||
match root.attribute(name) {
|
||||
@@ -1157,6 +1173,10 @@ pub fn parse(bytes: &[u8]) -> Result<Doc, Error> {
|
||||
for c in root.children() {
|
||||
p.node(c, &rst, &mut children, 1);
|
||||
}
|
||||
if p.work > p.budget {
|
||||
// Never a silently partial drawing.
|
||||
return Err(Error::Xml(format!("its <use> references expand to more than {} elements", p.budget)));
|
||||
}
|
||||
let transform = viewbox_transform(root, dw, dh);
|
||||
Ok(Doc {
|
||||
width: dw,
|
||||
|
||||
@@ -217,3 +217,63 @@ fn deep_nesting_errors_instead_of_overflowing_the_stack() {
|
||||
assert_eq!(crate::xml_depth(&s), 2);
|
||||
assert!(crate::parse(s.as_bytes()).is_ok());
|
||||
}
|
||||
|
||||
/// How deep `g`'s groups nest, and how many shapes it holds.
|
||||
fn depth_and_shapes(g: &Group) -> (usize, usize) {
|
||||
let mut depth = 0;
|
||||
let mut shapes = 0;
|
||||
for c in &g.children {
|
||||
match c {
|
||||
Node::Group(sub) => {
|
||||
let (d, s) = depth_and_shapes(sub);
|
||||
depth = depth.max(d + 1);
|
||||
shapes += s;
|
||||
}
|
||||
Node::Shape(_) => shapes += 1,
|
||||
Node::Image(_) => {}
|
||||
}
|
||||
}
|
||||
(depth, shapes)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn branching_use_references_stop_at_the_expansion_budget() {
|
||||
// Each group uses the one before twice: 30 levels are 2^30 copies of one rectangle, from a
|
||||
// file of a few kilobytes (#1236).
|
||||
let mut defs = String::from(r#"<g id="g0"><rect width="1" height="1"/></g>"#);
|
||||
for i in 1..=30 {
|
||||
defs += &format!(r##"<g id="g{i}"><use href="#g{p}"/><use href="#g{p}" x="1"/></g>"##, p = i - 1);
|
||||
}
|
||||
let svg = format!(r##"<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><defs>{defs}</defs><use href="#g30"/></svg>"##);
|
||||
let t0 = std::time::Instant::now();
|
||||
let e = parse(svg.as_bytes()).unwrap_err();
|
||||
assert!(e.to_string().contains("expand to more than"), "{e}");
|
||||
assert!(t0.elapsed() < std::time::Duration::from_secs(10), "{:?}", t0.elapsed());
|
||||
// Below the budget the same pattern imports whole: 2^10 rectangles.
|
||||
let small = svg.replace(r##"<use href="#g30"/></svg>"##, r##"<use href="#g10"/></svg>"##);
|
||||
let doc = parse(small.as_bytes()).unwrap();
|
||||
assert_eq!(depth_and_shapes(&doc.root).1, 1024);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mutually_referencing_uses_are_cut_at_the_first_repeat() {
|
||||
// A uses B and B uses A, neither inside the other: once each, not down to the depth limit.
|
||||
let svg = r##"<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><defs>
|
||||
<g id="a"><rect width="1" height="1"/><use href="#b"/></g>
|
||||
<g id="b"><rect width="2" height="2"/><use href="#a"/></g>
|
||||
</defs><use href="#a"/></svg>"##;
|
||||
let doc = parse(svg.as_bytes()).unwrap();
|
||||
let (depth, shapes) = depth_and_shapes(&doc.root);
|
||||
assert_eq!(shapes, 2, "a's and b's rectangle, once each");
|
||||
assert!(depth <= 6, "{depth}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_symbol_used_a_thousand_times_still_imports() {
|
||||
let uses: String = (0..1000).map(|i| format!(r##"<use href="#icon" x="{i}"/>"##)).collect();
|
||||
let svg = format!(
|
||||
r##"<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><defs><symbol id="icon"><rect width="1" height="1"/><circle r="1"/><ellipse rx="1" ry="2"/></symbol></defs>{uses}</svg>"##
|
||||
);
|
||||
let doc = parse(svg.as_bytes()).unwrap();
|
||||
assert_eq!(depth_and_shapes(&doc.root).1, 3000);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user