fix(svg): cap converted objects and nested images (#3564)

crates/svg/src/glyph.rs

@@ -43,7 +43,7 @@ pub fn glyph_art(document: &[u8], gid: u32, upem: f64, fill: Color, stroke: Opti
     let coloured = ["currentColor", "currentcolor", "context-fill", "context-stroke"].iter().any(|k| svg.contains(k));
     // usvg resolves `context-fill` and `context-stroke` only for markers.
     let svg = svg.replace("context-fill", &hex(fill)).replace("context-stroke", &stroke.map_or_else(|| "none".into(), hex));
-    let (doc, _) = crate::import::import(&svg, &ImportOptions::default()).ok()?;
+    let (doc, _) = crate::import::import(&svg, &ImportOptions::default(), crate::MAX_NODES).ok()?;
     let mut art = if whole {
         Node::group(NodeId(0), doc.layers.iter().map(|l| Arc::new(Node::group(NodeId(0), l.children().cloned().unwrap_or_default()))).collect())
     } else {

crates/svg/src/import.rs

@@ -20,8 +20,13 @@ use text::TextSlots;
 
 pub use depth::MAX_ELEMENT_DEPTH;
 pub(crate) use depth::xml_too_deep;
+pub use files::MAX_NESTED_SVG_IMAGES;
 pub(crate) use fx::BLEND;
 
+/// Most objects an imported SVG may expand to: a guard against `<use>` and nested-image
+/// expansion bombs (#3564).
+pub const MAX_NODES: usize = if cfg!(target_arch = "wasm32") { 500_000 } else { 2_000_000 };
+
 mod convert;
 mod importer;
 mod layers;
@@ -58,7 +63,7 @@ fn parse(svg: &str) -> Result<roxmltree::Document<'_>, SvgError> {
 }
 
 /// 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) -> Result<(Document, Vec<String>), SvgError> {
+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
     // recursion.
     let too_deep = || SvgError::TooDeep(depth::MAX_ELEMENT_DEPTH);
@@ -116,6 +121,10 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
         sublayers: found.sublayers,
         sublayer_groups: HashSet::new(),
         screen: k,
+        converted_nodes: 1,
+        max_nodes,
+        error: None,
+        nested_svg_images: 0,
     };
     im.doc.svg_scripts = found.scripts;
     if im.files.nested_text() {
@@ -183,7 +192,9 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
                 }
                 None => false,
             };
-            let id = im.doc.alloc_id();
+            let Some(id) = im.alloc_id() else {
+                return Err(im.error.unwrap_or(SvgError::TooLarge(im.max_nodes)));
+            };
             let preset = (im.doc.layers.len() % vectorcraft_doc::LAYER_COLORS.len()) as u8;
             let mut l = Node::layer(id, &name, LayerColor::Preset(preset));
             l.set_clips(clip);
@@ -197,10 +208,16 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
             im.keep_foreign(g.id(), &mut l);
             im.doc.layers.push(Arc::new(l));
         }
+        if let Some(err) = im.error.clone() {
+            return Err(err);
+        }
     } else {
         // An export of a one-layer document names the layer on the root element, its art (groups
         // among it) loose inside: the one layer takes that name.
         let children = im.children(top, base);
+        if let Some(err) = im.error.clone() {
+            return Err(err);
+        }
         if let Some(l) = im.doc.layers.first_mut() {
             let l = Arc::make_mut(l);
             if let Some(ch) = l.children_mut() {
@@ -211,6 +228,9 @@ pub(crate) fn import(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<S
             }
         }
     }
+    if let Some(err) = im.error {
+        return Err(err);
+    }
     if !im.sublayer_groups.is_empty() {
         for l in &mut im.doc.layers {
             promote_sublayers(Arc::make_mut(l), &im.sublayer_groups);
@@ -260,4 +280,12 @@ struct Importer {
     /// Points per screen pixel (the root's user unit before its `viewBox`): what a non-scaling
     /// stroke's width is measured in.
     screen: f64,
+    /// How many objects were converted so far.
+    converted_nodes: usize,
+    /// Most objects the document may expand to.
+    max_nodes: usize,
+    /// An error encountered while converting objects (e.g. [`SvgError::TooLarge`]).
+    error: Option<SvgError>,
+    /// How many nested SVG images were converted so far.
+    nested_svg_images: usize,
 }

crates/svg/src/import/files.rs

@@ -15,6 +15,9 @@ use vectorcraft_doc::LinkInfo;
 use super::{Edits, depth, href};
 use crate::{ImportOptions, MAX_ELEMENT_DEPTH, MAX_SVGZ, SvgError};
 
+/// Most nested SVG images an imported SVG may decode (#3564).
+pub const MAX_NESTED_SVG_IMAGES: usize = 256;
+
 /// The files an SVG's images link to.
 #[derive(Default)]
 pub(in crate::import) struct Files {
@@ -36,6 +39,10 @@ pub(in crate::import) struct Files {
     too_deep: AtomicBool,
     /// An SVGZ image was left out for unpacking past what the SVGZ images may.
     too_big: AtomicBool,
+    /// How many nested SVG images were decoded so far.
+    svg_images: AtomicUsize,
+    /// SVG images were left out for exceeding [`MAX_NESTED_SVG_IMAGES`].
+    too_many_svg_images: AtomicBool,
 }
 
 /// How usvg holds the bytes of a raster of one format (`ImageKind::PNG`…).
@@ -214,6 +221,9 @@ impl Files {
         if self.too_big.load(Ordering::Relaxed) {
             notes.push(format!("SVGZ images were left out: together they unpack to more than {} MB", MAX_SVGZ >> 20));
         }
+        if self.too_many_svg_images.load(Ordering::Relaxed) {
+            notes.push(format!("SVG images were left out: more than {MAX_NESTED_SVG_IMAGES} nested SVG images"));
+        }
         notes
     }
 
@@ -268,6 +278,9 @@ impl Files {
     /// can't read, when it isn't SVG or when its elements nest deeper than the SVGs around it
     /// leave: usvg reads it while it reads them, on the same stack.
     fn svg(&self, text: &str, opts: &usvg::Options) -> Option<ImageKind> {
+        if self.svg_images.fetch_add(1, Ordering::Relaxed) >= MAX_NESTED_SVG_IMAGES {
+            return leave_out(&self.too_many_svg_images, true);
+        }
         // Measured as the SVG holding it is (`import`): the text before the XML reader.
         let used = self.depth.load(Ordering::Relaxed);
         let left = MAX_ELEMENT_DEPTH.saturating_sub(used);

crates/svg/src/import/importer.rs

@@ -41,12 +41,27 @@ impl Importer {
     }
 
     pub(super) fn children(&mut self, g: &usvg::Group, acc: Affine) -> Vec<Arc<Node>> {
-        g.children().iter().filter_map(|c| self.node(c, acc)).map(Arc::new).collect()
+        if self.error.is_some() {
+            return vec![];
+        }
+        let mut out = Vec::new();
+        for c in g.children() {
+            if self.error.is_some() {
+                break;
+            }
+            if let Some(node) = self.node(c, acc) {
+                out.push(Arc::new(node));
+            }
+        }
+        out
     }
 
     /// The object usvg node `n` makes under transform `acc`, with what the file kept on its element
     /// (data, event and other applications' attributes); `None` for what imports as nothing.
     pub(super) fn node(&mut self, n: &usvg::Node, acc: Affine) -> Option<Node> {
+        if self.error.is_some() {
+            return None;
+        }
         let mut out = match n {
             usvg::Node::Group(g) => self.group(g, acc),
             usvg::Node::Path(p) => self.path(p, acc),
@@ -97,17 +112,33 @@ impl Importer {
         self.labels.get(id).map_or(id, String::as_str)
     }
 
+    /// Allocate a fresh node id within [`Importer::max_nodes`].
+    pub(super) fn alloc_id(&mut self) -> Option<NodeId> {
+        if self.error.is_some() {
+            return None;
+        }
+        if self.converted_nodes >= self.max_nodes {
+            self.error = Some(SvgError::TooLarge(self.max_nodes));
+            return None;
+        }
+        self.converted_nodes += 1;
+        Some(self.doc.alloc_id())
+    }
+
     /// A new object named after element `id` (unless it has none or a made-up one).
-    fn named(&mut self, id: &str, kind: NodeKind) -> Node {
-        let nid = self.doc.alloc_id();
+    fn named(&mut self, id: &str, kind: NodeKind) -> Option<Node> {
+        let nid = self.alloc_id()?;
         let mut n = Node::new(nid, kind);
         if !id.is_empty() && !made_up(id) {
             n.name = Some(self.name_of(id).to_string());
         }
-        n
+        Some(n)
     }
 
     fn group(&mut self, g: &usvg::Group, acc: Affine) -> Option<Node> {
+        if self.error.is_some() {
+            return None;
+        }
         let url = self.links.get(g.id()).cloned();
         let mut n = match self.instance(g, acc) {
             Some(n) => n,
@@ -143,7 +174,7 @@ impl Importer {
         let art = match kids.len() {
             0 => return None,
             1 => Arc::unwrap_or_clone(kids.pop()?),
-            _ => self.named("", NodeKind::Group { children: kids, clip: false }),
+            _ => self.named("", NodeKind::Group { children: kids, clip: false })?,
         };
         // usvg clips a `<use>` of a symbol to its viewport: that only counts when it cuts the art.
         let whole = g.clip_path().is_none_or(|cp| uncut(cp, ts, &art, xf));
@@ -164,7 +195,7 @@ impl Importer {
         };
         let label = if made_up(g.id()) { "a symbol instance".to_string() } else { format!("'{}'", g.id()) };
         let mask = g.mask().and_then(|m| self.opacity_mask(m, ts, &label));
-        let mut n = self.named(g.id(), NodeKind::SymbolInstance { symbol: name, xf });
+        let mut n = self.named(g.id(), NodeKind::SymbolInstance { symbol: name, xf })?;
         n.opacity = g.opacity().get();
         n.blend = blend(g.blend_mode());
         n.isolate = g.isolate();
@@ -227,7 +258,7 @@ impl Importer {
             ap
         });
         text::fold_scale(&mut obj, grow);
-        let mut n = self.named(&name, NodeKind::Text(Box::new(obj)));
+        let mut n = self.named(&name, NodeKind::Text(Box::new(obj)))?;
         if let Some(ap) = under {
             n.appearance = ap.into();
         }
@@ -277,12 +308,12 @@ impl Importer {
         let mut n = if let Some(cp) = g.clip_path() {
             // SVG filters before it clips: the effects go on the art inside the clip.
             if !effects.is_empty() {
-                let mut inner = self.named("", NodeKind::Group { children, clip: false });
+                let mut inner = self.named("", NodeKind::Group { children, clip: false })?;
                 inner.appearance.effects = effects.clone();
                 children = vec![Arc::new(inner)];
             }
             let children = self.clipped(cp, ts, children, &label)?;
-            self.named(id, NodeKind::Group { children, clip: true })
+            self.named(id, NodeKind::Group { children, clip: true })?
         } else {
             if children.is_empty() {
                 return None;
@@ -310,7 +341,7 @@ impl Importer {
                 c.appearance.effects.extend(effects);
                 return Some(c);
             }
-            let mut n = self.named(id, NodeKind::Group { children, clip: false });
+            let mut n = self.named(id, NodeKind::Group { children, clip: false })?;
             n.appearance.effects = effects;
             n
         };
@@ -358,7 +389,7 @@ impl Importer {
         // over when clipped, white when not, inverted when the mask is).
         let art = match children.len() {
             1 => Arc::unwrap_or_clone(children.pop()?),
-            _ => self.named("", NodeKind::Group { children, clip: false }),
+            _ => self.named("", NodeKind::Group { children, clip: false })?,
         };
         let mut mask = vectorcraft_doc::OpacityMask::new(art, clip);
         mask.invert = invert;
@@ -384,7 +415,7 @@ impl Importer {
         for clip in clips {
             let mut ch = vec![Arc::new(clip)];
             ch.extend(children);
-            children = vec![Arc::new(self.named("", NodeKind::Group { children: ch, clip: true }))];
+            children = vec![Arc::new(self.named("", NodeKind::Group { children: ch, clip: true })?)];
         }
         let mut ch = vec![Arc::new(outer)];
         ch.extend(children);
@@ -400,7 +431,9 @@ impl Importer {
         if let NodeKind::Layer { children, clip: clip @ true, .. } = &mut l.kind {
             *clip = false;
             let clipped = std::mem::take(children);
-            children.push(Arc::new(self.named("", NodeKind::Group { children: clipped, clip: true })));
+            if let Some(group) = self.named("", NodeKind::Group { children: clipped, clip: true }) {
+                children.push(Arc::new(group));
+            }
         }
         if let Some(ch) = l.children_mut() {
             let i = index.min(ch.len());
@@ -421,16 +454,15 @@ impl Importer {
         let id = cp.id().to_string();
         if subs.len() == 1 {
             let path = PathData::new(subs);
-            return Some(self.named(&id, NodeKind::Path { path, rule: r, live: None, clipping: true, guide: false }));
+            return self.named(&id, NodeKind::Path { path, rule: r, live: None, clipping: true, guide: false });
         }
-        let children = subs
-            .into_iter()
-            .map(|sp| {
-                let nid = self.doc.alloc_id();
-                Arc::new(Node::new(nid, NodeKind::Path { path: PathData::single(sp), rule: r, live: None, clipping: false, guide: false }))
-            })
-            .collect();
-        Some(self.named(&id, NodeKind::Compound { children, rule: r }))
+        let mut children = Vec::with_capacity(subs.len());
+        for sp in subs {
+            let nid = self.alloc_id()?;
+            children
+                .push(Arc::new(Node::new(nid, NodeKind::Path { path: PathData::single(sp), rule: r, live: None, clipping: false, guide: false })));
+        }
+        self.named(&id, NodeKind::Compound { children, rule: r })
     }
 
     /// A usvg paint mapped by `m`, for art covering `area` (after `m`: where a reflected or
@@ -502,9 +534,11 @@ impl Importer {
         }
         let clip =
             self.named("", NodeKind::Path { path: shapes::rectangle(tile), rule: FillRule::NonZero, live: None, clipping: true, guide: false });
+        let Some(clip) = clip else { return Paint::None };
         let mut children = vec![Arc::new(clip)];
         children.extend(content);
         let art = self.named("", NodeKind::Group { children, clip: true });
+        let Some(art) = art else { return Paint::None };
         let base = if pt.id().is_empty() { "Pattern" } else { pt.id() };
         let name = unique_name(base, |n| self.doc.pattern(n).is_some());
         let mut def = PatternDef::new(&name, vec![Arc::new(art)]);
@@ -575,19 +609,16 @@ impl Importer {
         let appearance: vectorcraft_doc::Shared<Appearance> = appearance.into();
         let mut n = if path.subpaths.len() > 1 {
             // Multi-subpath SVG paths are compound paths (as in Illustrator).
-            let children = path
-                .subpaths
-                .into_iter()
-                .map(|sp| {
-                    let id = self.doc.alloc_id();
-                    let mut c = Node::new(id, NodeKind::Path { path: PathData::single(sp), rule: r, live: None, clipping: false, guide: false });
-                    c.appearance = appearance.clone();
-                    Arc::new(c)
-                })
-                .collect();
-            self.named(p.id(), NodeKind::Compound { children, rule: r })
+            let mut children = Vec::with_capacity(path.subpaths.len());
+            for sp in path.subpaths {
+                let id = self.alloc_id()?;
+                let mut c = Node::new(id, NodeKind::Path { path: PathData::single(sp), rule: r, live: None, clipping: false, guide: false });
+                c.appearance = appearance.clone();
+                children.push(Arc::new(c));
+            }
+            self.named(p.id(), NodeKind::Compound { children, rule: r })?
         } else {
-            self.named(p.id(), NodeKind::Path { path, rule: r, live: None, clipping: false, guide: false })
+            self.named(p.id(), NodeKind::Path { path, rule: r, live: None, clipping: false, guide: false })?
         };
         n.appearance = appearance;
         Some(n)
@@ -620,15 +651,23 @@ impl Importer {
             if link.is_some() && !missing { blob.with_proxy() } else { blob }
         });
         let xf = acc * Affine::scale_non_uniform(size.width() as f64 / pw as f64, size.height() as f64 / ph as f64);
-        Some(self.named(
+        self.named(
             i.id(),
             NodeKind::Image(Box::new(ImageObject { key, width: pw, height: ph, xf, link, placement: Default::default(), pixels: None })),
-        ))
+        )
     }
 
     /// An SVG shown as an image (a data URL or a linked file) → its art, `acc` mapping its size.
     /// Its ids are its own: they mean nothing in the SVG around it.
     fn svg_image(&mut self, tree: &usvg::Tree, acc: Affine) -> Option<Node> {
+        if self.error.is_some() {
+            return None;
+        }
+        if self.nested_svg_images >= crate::MAX_NESTED_SVG_IMAGES {
+            self.error = Some(SvgError::TooLarge(self.max_nodes));
+            return None;
+        }
+        self.nested_svg_images += 1;
         let outer = (
             std::mem::take(&mut self.links),
             std::mem::take(&mut self.hidden),

crates/svg/src/lib.rs

@@ -333,7 +333,7 @@ pub struct LinkedImage {
     pub bytes: std::sync::Arc<Vec<u8>>,
 }
 
-#[derive(Debug, thiserror::Error)]
+#[derive(Clone, Debug, thiserror::Error)]
 pub enum SvgError {
     #[error("SVG parse error: {0}")]
     Parse(String),
@@ -343,6 +343,10 @@ pub enum SvgError {
     /// SVGZ data unpacks to more than the reader takes (a guard against decompression bombs).
     #[error("SVG parse error: {0}")]
     TooBig(String),
+    /// The SVG expands to more objects than the reader takes (a guard against `<use>` and
+    /// nested-image expansion bombs, #3564).
+    #[error("the SVG expands to more than {0} objects")]
+    TooLarge(usize),
 }
 
 /// Export a document as an SVG string (linked images are not returned: see [`export_full`]).
@@ -541,13 +545,18 @@ pub fn import_with_report(svg: &str) -> Result<(Document, Vec<String>), SvgError
 }
 
 pub use glyph::glyph_art;
-pub use import::{MAX_ELEMENT_DEPTH, MAX_FOREIGN, MAX_FOREIGN_VALUE};
+pub use import::{MAX_ELEMENT_DEPTH, MAX_FOREIGN, MAX_FOREIGN_VALUE, MAX_NESTED_SVG_IMAGES, MAX_NODES};
 
 /// Import an SVG document reading the files it links to as `opts` says, also returning warnings.
 pub fn import_with(svg: &str, opts: &ImportOptions) -> Result<(Document, Vec<String>), SvgError> {
+    import_within(svg, opts, MAX_NODES)
+}
+
+/// [`import_with`], capping the objects the document may expand to at `max_nodes`.
+pub fn import_within(svg: &str, opts: &ImportOptions, max_nodes: usize) -> Result<(Document, Vec<String>), SvgError> {
     // usvg converts the tree by recursion, in large stack frames: the nesting it is given is
     // capped (`import::depth`), and it gets room for that cap whatever thread asks.
-    vectorcraft_doc::depth::with_deep_stack(|| import::import(svg, opts))
+    vectorcraft_doc::depth::with_deep_stack(|| import::import(svg, opts, max_nodes))
 }
 
 /// Reads a linked file: its bytes and the link to it (absolute path, size, time and hash), or

crates/svg/tests/it/import_details.rs

@@ -6,7 +6,7 @@ use std::path::{Path, PathBuf};
 
 use vectorcraft_doc::{Document, ImageObject, LinkInfo, Node, NodeKind};
 use vectorcraft_geom::Rect;
-use vectorcraft_svg::{ImportOptions, import_with, import_with_report};
+use vectorcraft_svg::{ImportOptions, MAX_NESTED_SVG_IMAGES, SvgError, import_with, import_with_report, import_within};
 use vectorcraft_testkit::raster::{Image, assert_similar, render_artboard};
 
 fn svg(body: &str) -> String {
@@ -372,3 +372,54 @@ fn inkscape_pages_become_artboards() {
 fn svg_doc() -> String {
     svg(r#"<rect x="10" y="10" width="20" height="20"/>"#)
 }
+
+/// An ordinary nested SVG image imports within budget, while `<use>`-of-nested-image fan-out
+/// past the node budget is refused with `SvgError::TooLarge` (#3564).
+#[test]
+fn nested_image_within_budget_imports_and_use_fan_out_is_refused() {
+    let inner = r#"<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20"><rect width="10" height="10" fill="red"/></svg>"#;
+    let nested = format!("data:image/svg+xml;base64,{}", vectorcraft_svg::base64_encode(inner.as_bytes()));
+    let single = format!(r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><image href="{nested}" width="20" height="20"/></svg>"#);
+    // An ordinary nested image imports within a small budget (3 objects: Layer 1, Group, Rect).
+    let (d, _) = import_within(&single, &ImportOptions::default(), 10).unwrap();
+    assert!(!d.layers[0].children().unwrap().is_empty());
+
+    // Fan-out duplicating the nested image via <use> exceeds the budget and is refused.
+    let uses = "<use href=\"#item\"/>".repeat(20);
+    let fanout = format!(
+        r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">
+            <defs><g id="item"><image href="{nested}" width="20" height="20"/></g></defs>
+            {uses}
+        </svg>"#
+    );
+    let err = import_within(&fanout, &ImportOptions::default(), 5).unwrap_err();
+    assert!(matches!(err, SvgError::TooLarge(5)), "{err:?}");
+    assert_eq!(err.to_string(), "the SVG expands to more than 5 objects");
+
+    // Highly amplified multi-level <use> tree is refused without exploding memory.
+    let multi_level = format!(
+        r##"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">
+            <defs>
+                <g id="g0"><image href="{nested}" width="20" height="20"/></g>
+                <g id="g1"><use href="#g0"/><use href="#g0"/></g>
+                <g id="g2"><use href="#g1"/><use href="#g1"/></g>
+                <g id="g3"><use href="#g2"/><use href="#g2"/></g>
+                <g id="g4"><use href="#g3"/><use href="#g3"/></g>
+            </defs>
+            <use href="#g4"/>
+        </svg>"##
+    );
+    let err = import_within(&multi_level, &ImportOptions::default(), 15).unwrap_err();
+    assert!(matches!(err, SvgError::TooLarge(15)), "{err:?}");
+}
+
+/// Nested SVG images exceeding `MAX_NESTED_SVG_IMAGES` are left out with a warning note (#3564).
+#[test]
+fn nested_svg_images_past_count_cap_are_left_out() {
+    let inner = r#"<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><rect width="5" height="5"/></svg>"#;
+    let nested = format!("data:image/svg+xml;base64,{}", vectorcraft_svg::base64_encode(inner.as_bytes()));
+    let images = format!(r#"<image href="{nested}" width="10" height="10"/>"#).repeat(MAX_NESTED_SVG_IMAGES + 10);
+    let svg = format!(r#"<svg xmlns="http://www.w3.org/2000/svg" width="500" height="500">{images}</svg>"#);
+    let (_d, warnings) = import_with(&svg, &ImportOptions::default()).unwrap();
+    assert!(warnings.iter().any(|w| w.contains(&format!("more than {MAX_NESTED_SVG_IMAGES} nested SVG images"))));
+}