svg import: cap attributes per element, map span offsets to characters once

crates/svg/src/import.rs

@@ -704,13 +704,28 @@ fn simple_hash(b: &[u8]) -> String {
 /// Deepest element nesting the parser is given: deeper documents would exhaust the stack.
 const MAX_NESTING: usize = 512;
 
+/// The most attributes one element may have. The XML parser checks each attribute against the
+/// earlier ones, so a huge count is quadratic work; real elements have a few dozen at most.
+const MAX_ATTRIBUTES: usize = 4096;
+
 /// How deep elements nest in `data`: a scan of its tags (comments, CDATA, declarations and quoted
 /// attribute values skipped) that stops once past `limit`.
 pub(crate) fn nesting(data: &[u8], limit: usize) -> usize {
+    scan(data, limit).0
+}
+
+/// Whether an element of `data` has more than [`MAX_ATTRIBUTES`] attributes (counted as quoted
+/// values, before the parser sees them).
+pub(crate) fn too_wide(data: &[u8]) -> bool {
+    scan(data, usize::MAX).1 > MAX_ATTRIBUTES
+}
+
+/// [`nesting`] and the most attribute values on one element; stops once either is past its limit.
+fn scan(data: &[u8], limit: usize) -> (usize, usize) {
     let find = |from: usize, pat: &[u8]| -> usize {
         data.get(from..).and_then(|r| r.windows(pat.len()).position(|w| w == pat)).map_or(data.len(), |p| from + p + pat.len())
     };
-    let (mut depth, mut max, mut i) = (0usize, 0usize, 0usize);
+    let (mut depth, mut max, mut i, mut widest) = (0usize, 0usize, 0usize, 0usize);
     while let Some(off) = data.get(i..).and_then(|r| r.iter().position(|b| *b == b'<')) {
         i += off;
         let rest = data.get(i..).unwrap_or(&[]);
@@ -725,12 +740,15 @@ pub(crate) fn nesting(data: &[u8], limit: usize) -> usize {
             i = find(i + 2, b">");
         } else {
             // A start tag: to its `>` outside quotes; `/>` closes it.
-            let (mut quote, mut j, mut closed) = (None, i + 1, false);
+            let (mut quote, mut j, mut closed, mut attrs) = (None, i + 1, false, 0usize);
             while let Some(&c) = data.get(j) {
                 match quote {
                     Some(q) if c == q => quote = None,
                     Some(_) => {}
-                    None if c == b'"' || c == b'\'' => quote = Some(c),
+                    None if c == b'"' || c == b'\'' => {
+                        quote = Some(c);
+                        attrs += 1;
+                    }
                     None if c == b'>' => {
                         closed = data.get(j.wrapping_sub(1)) == Some(&b'/');
                         break;
@@ -739,17 +757,21 @@ pub(crate) fn nesting(data: &[u8], limit: usize) -> usize {
                 }
                 j += 1;
             }
+            widest = widest.max(attrs);
+            if widest > MAX_ATTRIBUTES {
+                return (max, widest);
+            }
             if !closed {
                 depth += 1;
                 max = max.max(depth);
                 if max > limit {
-                    return max;
+                    return (max, widest);
                 }
             }
             i = j + 1;
         }
     }
-    max
+    (max, widest)
 }
 
 /// Whether `data` nests elements more than [`MAX_NESTING`] deep: the byte scan first, then the
@@ -883,6 +905,9 @@ fn import_within(
 ) -> Result<Imported, ImportError> {
     // SVGZ is unpacked here, within a budget, so every check below sees the real document.
     let data = budget::document(data, &limits).map_err(ImportError)?;
+    if too_wide(&data) {
+        return Err(ImportError(format!("an element has more than {MAX_ATTRIBUTES} attributes")));
+    }
     if too_deep(&data) {
         return Err(ImportError(format!("elements nested more than {MAX_NESTING} deep")));
     }

crates/svg/src/import/budget.rs

@@ -87,7 +87,7 @@ fn embedded(used: &Used, limits: &Limits, data: &[u8], opts: &Options) -> Option
         }
     };
     used.bytes.fetch_add(plain.len(), Ordering::Relaxed);
-    if super::too_deep(&plain) {
+    if super::too_wide(&plain) || super::too_deep(&plain) {
         return None;
     }
     used.depth.fetch_add(1, Ordering::Relaxed);

crates/svg/src/import/hostile_tests.rs

@@ -181,3 +181,15 @@ fn entity_expansion_cannot_bypass_the_nesting_limit() {
     let err = import_svg(&mut d, data.as_bytes(), p, 0.0, 0.0, "x").err().map(|e| e.to_string()).unwrap_or_default();
     assert!(err.contains("nested more than 512 deep"), "{err:?}");
 }
+
+#[test]
+fn an_element_with_thousands_of_attributes_is_refused_before_parsing() {
+    use super::too_wide;
+    let many = |n: usize| (0..n).map(|i| format!(" a{i}=\"1\"")).collect::<String>();
+    assert!(!too_wide(format!("<svg{}/>", many(4096)).as_bytes()));
+    assert!(too_wide(format!("<svg><g{}/></svg>", many(4097)).as_bytes()));
+    assert!(!too_wide(format!("<svg>{}</svg>", "<g a='1' b='2'/>".repeat(10_000)).as_bytes()), "per element, not per file");
+    let mut d = Document::new("t");
+    let p = d.first_page().unwrap();
+    assert!(import_svg(&mut d, &svg(&format!("<g{}/>", many(100_000))), p, 0.0, 0.0, "x").is_err());
+}

crates/svg/src/import/text.rs

@@ -195,7 +195,7 @@ pub(crate) fn text(cx: &mut Ctx, t: &usvg::Text, parent: NodeId, base: Affine, d
         let x = chunk.x().map_or(pen_x, f64::from) + d(t.dx());
         let y = chunk.y().map_or(pen_y, f64::from) + d(t.dy());
         char_at += count;
-        let ci = |b: usize| text.get(..b).map_or(count, |p| p.chars().count());
+        let ci = char_index(text);
         let spans: Vec<_> = chunk.spans().iter().map(|sp| (ci(sp.start()), ci(sp.end()), sp, span_style(sp, s, fonts))).collect();
         if spans.is_empty() {
             continue;
@@ -229,6 +229,13 @@ pub(crate) fn text(cx: &mut Ctx, t: &usvg::Text, parent: NodeId, base: Affine, d
     }
 }
 
+/// The characters of `text` before a byte offset (all of them for an offset that is past the end
+/// or inside a character), by a search in a table built once: many spans don't rescan the prefix.
+fn char_index(text: &str) -> impl Fn(usize) -> usize + '_ {
+    let starts: Vec<usize> = text.char_indices().map(|(i, _)| i).collect();
+    move |b| if text.is_char_boundary(b) { starts.partition_point(|s| *s < b) } else { starts.len() }
+}
+
 /// The text of `lines` (one per line, stacked at their pitch; `s`: their scale): characters, the
 /// first span's style and a run per span, fills still to come.
 fn text_data(lines: &[Line], s: f64) -> TextData {
@@ -262,8 +269,7 @@ fn on_path(cx: &mut Ctx, t: &usvg::Text, world: Affine, parent: NodeId) -> bool
     let Some(len) = figcraft_geom::measure::polylines(&path).and_then(|l| l.first().map(|l| l.length())).filter(|l| *l > 1e-9) else { return false };
     let fonts = cx.fonts.get_or_insert_with(Fonts::load);
     let text = chunk.text();
-    let count = text.chars().count();
-    let ci = |b: usize| text.get(..b).map_or(count, |p| p.chars().count());
+    let ci = char_index(text);
     let spans: Vec<_> = chunk.spans().iter().map(|sp| (ci(sp.start()), ci(sp.end()), sp, span_style(sp, 1.0, fonts))).collect();
     let line = Line { x: 0.0, y: 0.0, anchor: chunk.anchor(), text: text.to_string(), spans };
     let mut td = text_data(std::slice::from_ref(&line), 1.0);

crates/svg/src/import/text_tests.rs

@@ -30,3 +30,11 @@ fn css_system_font_names_keep_their_name_where_the_system_font_is() {
     assert_eq!(family(&named(&["-apple-system", "BlinkMacSystemFont", "system-ui", "sans-serif"]), &elsewhere), "Inter");
     assert_eq!(family(&named(&["-apple-system"]), &elsewhere), "-apple-system", "nothing installed: the first name it gives");
 }
+
+#[test]
+fn char_index_counts_the_characters_before_a_byte_offset() {
+    let t = "aé😀b";
+    let ci = super::char_index(t);
+    assert_eq!([ci(0), ci(1), ci(3), ci(7), ci(8)], [0, 1, 2, 3, 4]);
+    assert_eq!([ci(2), ci(4), ci(99)], [4, 4, 4], "inside a character or past the end: all of them");
+}