xfa, xfdf: refuse XML nested past 1024 levels instead of aborting the process (#987)

* xfa, xfdf: refuse XML nested past 1024 levels (roxmltree recurses with nesting depth)

roxmltree parses by recursive descent, so its stack use grows with the nesting depth of the
document it is handed. Ten thousand nested elements overflow the main thread's stack and abort
the process ("fatal runtime error: stack overflow, aborting", SIGABRT, core dumped); three
thousand still parse on the default stack. A stack overflow is `abort()`, so `catch_unwind`
cannot intercept it and the whole application dies.

Two of our XML entry points hand attacker-supplied text straight to it:

- XFDF/FDF form data import. A 1.4 MB file with 50,000 nested `<field>` elements aborted
  `pdfcraft-cli run doc_import_data` in 288 ms (SIGABRT, core dumped). The same file now returns
  "the file nests XML elements 50003 deep, past the 1024 the reader accepts" in 54 ms, at 19 MB.
- The XFA template parser (`crates/xfa/src/parse.rs`), reached when a document with an XFA form
  is opened or laid out. A 10,000-deep template aborted this crate's own test run before the fix.

`crates/xfa/src/data.rs` parses the datasets packet and the data written back into it, so it is
guarded the same way.

The bound has to be applied before parsing, not while walking the result: the overflow happens
inside the parser, so a depth cap on our own walks (64, unchanged) cannot prevent it. Both
crates measure the nesting with a small scanner that skips comments, CDATA, processing
instructions, the DOCTYPE and quoted attribute values, and counts a tag that closes itself
(`<draw/>`) as one level while it is read rather than as a level left open, so a template full of
styling elements is two levels deep rather than two thousand. It cannot under-count a real
element; a stray `<` in text is invalid XML and only makes it over-count, so it errs towards
refusing a file rather than trusting it.

The limit is 1024 because of measurement, not taste: 3000 levels still parse on the default stack
and 10000 abort, and `hostile_templates_fail_without_panicking` keeps asserting that a 500-deep
template parses, so the bound has to sit well above that.

Tests: `deeply_nested_fields_are_refused_instead_of_overflowing_the_stack` and
`deeply_nested_template_is_refused_instead_of_overflowing_the_stack` both abort with a stack
overflow before this change and pass after it; `nesting_depth_scanner_counts_what_matters` and
`self_closing_elements_do_not_accumulate_in_the_depth` cover the scanner (comments, CDATA, a
quoted attribute holding `<`, and 2,000 sibling `<draw/>` elements, which a scanner that leaves
self-closing tags open counts as 2,000 deep);
`many_self_closing_elements_do_not_accumulate_into_the_bound` parses a template holding two
thousand of them; and `nested_fields_within_the_limit_still_read` proves a 16-level field tree
still reads with the same dotted names. Both suites pass: xfa 36, xfdf 18.

The parse options also carry `nodes_limit` now, which was already the shape the XFA parser used:
a 93 MB XFDF with two million sibling elements went from 1082 MB and 2323 ms to 289 MB and
1299 ms ("nodes limit reached").

The scanner is duplicated between the two crates because nothing between them owns XML. If it
should live in one place, say where and it moves.

* xfa: the depth limit holds on a default-stack worker thread

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>

crates/xfa/src/data.rs

@@ -73,6 +73,9 @@ fn to_node(n: roxmltree::Node, depth: usize) -> DataNode {
 /// The `xfa:data` element of the datasets packet in `xdp` (or a bare datasets document), as a
 /// tree whose root is that element.
 pub fn parse_datasets(xdp: &str) -> Option<DataNode> {
+    if crate::xml_nesting_depth(xdp) > crate::MAX_XML_DEPTH {
+        return None;
+    }
     let opts = roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: MAX_NODES };
     let doc = roxmltree::Document::parse_with_options(xdp, opts).ok()?;
     let datasets = doc.descendants().find(|n| n.is_element() && n.tag_name().name() == "datasets")?;
@@ -584,6 +587,10 @@ fn merge(text: &str, within: Within, op: Op, warnings: &mut Vec<String>) -> Merg
         Op::Values(w) => w,
         Op::Remove(_) => &[],
     };
+    if crate::xml_nesting_depth(text) > crate::MAX_XML_DEPTH {
+        warnings.push("The XFA data nests XML elements deeper than the reader accepts; the field values were not written to it".into());
+        return Merged::Failed;
+    }
     let opts = || roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: MAX_NODES };
     // A packet of an array may use the prefixes the XDP envelope declares: read it inside
     // an element declaring the usual ones.

crates/xfa/src/lib.rs

@@ -20,6 +20,75 @@ pub mod text;
 
 use pdfcraft_cos::Document;
 
+/// The deepest element nesting accepted before an XML document reaches the parser.
+///
+/// roxmltree parses by recursive descent, so its stack use grows with the nesting depth: ten
+/// thousand levels overflow the main thread's stack, and a stack overflow is `abort()`, which no
+/// `catch_unwind` can intercept. The bound therefore has to be checked before parsing, not while
+/// walking the result. Around three thousand levels still parse on the default stack, so this stays
+/// clear of the cliff while leaving far more room than the walks below, which cut off at 64 - and
+/// more than the five hundred levels `hostile_templates_fail_without_panicking` keeps parsing.
+/// `pdfcraft-xfdf` carries the same check for form data files.
+pub(crate) const MAX_XML_DEPTH: usize = 1_024;
+
+/// Element nesting depth of `text`, skipping comments, CDATA, processing instructions, the DOCTYPE
+/// and quoted attribute values. A tag that closes itself (`<draw/>`) is one level while it is read
+/// and does not stay open, so a long run of styling elements cannot accumulate into the bound. It
+/// cannot under-count a real element; a stray `<` in text is invalid XML and only makes it
+/// over-count, which errs towards refusing the document.
+pub(crate) fn xml_nesting_depth(text: &str) -> usize {
+    fn find(hay: &[u8], from: usize, needle: &[u8]) -> Option<usize> {
+        (from..hay.len()).find(|&p| hay[p..].starts_with(needle))
+    }
+    let b = text.as_bytes();
+    let (mut i, mut depth, mut max) = (0usize, 0usize, 0usize);
+    while i < b.len() {
+        if b[i] != b'<' {
+            i += 1;
+        } else if b[i..].starts_with(b"<!--") {
+            i = find(b, i + 4, b"-->").map_or(b.len(), |p| p + 3);
+        } else if b[i..].starts_with(b"<![CDATA[") {
+            i = find(b, i + 9, b"]]>").map_or(b.len(), |p| p + 3);
+        } else if b[i..].starts_with(b"<?") {
+            i = find(b, i + 2, b"?>").map_or(b.len(), |p| p + 2);
+        } else if b[i..].starts_with(b"<!") {
+            i = find(b, i + 2, b">").map_or(b.len(), |p| p + 1);
+        } else if b[i..].starts_with(b"</") {
+            depth = depth.saturating_sub(1);
+            i = find(b, i + 2, b">").map_or(b.len(), |p| p + 1);
+        } else {
+            depth += 1;
+            max = max.max(depth);
+            // Skip the rest of the start tag, over any quoted attribute value, and remember its last
+            // significant byte: a tag that ends in `/` closes itself, so it is one level deep while it
+            // is read rather than a level left open.
+            i += 1;
+            let mut quote = 0u8;
+            let mut last = 0u8;
+            while i < b.len() {
+                let c = b[i];
+                if quote != 0 {
+                    if c == quote {
+                        quote = 0;
+                    }
+                } else if c == b'"' || c == b'\'' {
+                    quote = c;
+                } else if c == b'>' {
+                    i += 1;
+                    break;
+                } else if !c.is_ascii_whitespace() {
+                    last = c;
+                }
+                i += 1;
+            }
+            if last == b'/' {
+                depth -= 1;
+            }
+        }
+    }
+    max
+}
+
 pub use data::{
     DataNode, DataOp, DataPath, DatasetsWrite, FieldData, FieldDatum, add_data_instances, build_data, data_path, iso_to_pattern, parse_datasets,
     pattern_to_iso, read_values, read_values_with, remove_data_instance, som_to_path, write_data_ops, write_data_value, write_datasets,

crates/xfa/src/parse.rs

@@ -640,6 +640,13 @@ fn collect_page_roles(nodes: &[Node], out: &mut Vec<(String, PageRole)>, depth:
 
 /// Parse a template packet. Returns the template and the warnings met on the way.
 pub fn parse(xml: &str) -> Result<(Template, Vec<String>), XfaError> {
+    let depth = crate::xml_nesting_depth(xml);
+    if depth > crate::MAX_XML_DEPTH {
+        return Err(XfaError::Malformed(format!(
+            "the template nests XML elements {depth} deep, past the {} the reader accepts",
+            crate::MAX_XML_DEPTH
+        )));
+    }
     let opts = roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: MAX_NODES };
     let doc = roxmltree::Document::parse_with_options(xml, opts).map_err(|e| XfaError::Malformed(format!("template XML: {e}")))?;
     let root = doc.root_element();

crates/xfa/src/tests.rs

@@ -1110,6 +1110,61 @@ fn data_paths_leave_areas_out() {
     assert_eq!(data_path(&tpl, ""), Vec::new());
 }
 
+/// A template nested thousands of elements deep must be refused. roxmltree recurses with the
+/// nesting depth, so it used to overflow the stack - and a stack overflow aborts the process,
+/// which no `catch_unwind` can intercept.
+#[test]
+fn deeply_nested_template_is_refused_instead_of_overflowing_the_stack() {
+    let mut xml = String::from(r#"<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/">"#);
+    for i in 0..10_000 {
+        xml.push_str(&format!(r#"<subform name="s{i}">"#));
+    }
+    xml.push_str(&"</subform>".repeat(10_000));
+    xml.push_str("</template>");
+    assert!(matches!(crate::parse::parse(&xml), Err(crate::XfaError::Malformed(_))), "ten thousand nested subforms must be refused, not parsed");
+}
+
+/// The depth scanner skips what is not an element and cannot under-count one.
+#[test]
+fn nesting_depth_scanner_counts_what_matters() {
+    use crate::xml_nesting_depth as d;
+    assert_eq!(d(""), 0);
+    assert_eq!(d("<a><b><c/></b></a>"), 3);
+    assert_eq!(d(r#"<?xml version="1.0"?><!-- <x> --><a/>"#), 1);
+    assert_eq!(d("<a><![CDATA[<not-an-element>]]></a>"), 1);
+    assert_eq!(d(r#"<a b="<"><c/></a>"#), 2);
+    assert_eq!(d("<a/>"), 1);
+    assert_eq!(d("<a><b/></a>"), 2);
+    assert_eq!(d(&"<empty/>".repeat(2_000)), 1, "elements that close themselves must not accumulate");
+}
+
+/// A template holding thousands of self-closing elements is two levels deep, not thousands. Before
+/// the scanner knew that, a form with a lot of styling elements was refused as too deeply nested.
+#[test]
+fn many_self_closing_elements_do_not_accumulate_into_the_bound() {
+    let mut xml = String::from(r#"<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/"><subform layout="tb">"#);
+    for i in 0..2_000 {
+        xml.push_str(&format!(r#"<draw name="d{i}"/>"#));
+    }
+    xml.push_str("</subform></template>");
+    assert!(crate::parse::parse(&xml).is_ok(), "two thousand self-closing draws are two levels deep, not two thousand");
+}
+
+/// The bound must hold on a worker thread too, whose default stack (2 MiB) is a quarter of the
+/// main thread's: a template exactly at the limit parses there without overflowing.
+#[test]
+fn template_at_the_depth_limit_parses_on_a_worker_thread() {
+    let depth = crate::MAX_XML_DEPTH - 1;
+    let mut xml = String::from(r#"<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/">"#);
+    for i in 0..depth {
+        xml.push_str(&format!(r#"<subform name="s{i}">"#));
+    }
+    xml.push_str(&"</subform>".repeat(depth));
+    xml.push_str("</template>");
+    let parsed = std::thread::spawn(move || crate::parse::parse(&xml).is_ok()).join();
+    assert!(matches!(parsed, Ok(true)), "a template at the limit parses on a default-stack thread");
+}
+
 /// A form of one page area (7.5in × 10in of content from 0.5in) around `body`.
 fn one_area_form(root_attrs: &str, body: &str) -> String {
     format!(

crates/xfdf/src/lib.rs

@@ -67,6 +67,92 @@ pub struct Report {
 
 // ── names ───────────────────────────────────────────────────────────────────────────────────
 
+/// Form data files are attacker-supplied. Refuse a DTD and bound the node count, exactly as the
+/// XFA template parser does (`crates/xfa/src/parse.rs`), and bound how deep a nested field tree
+/// may go: the walk below recurses once per nesting level.
+const MAX_NODES: u32 = 2_000_000;
+const MAX_DEPTH: usize = 64;
+
+/// The deepest element nesting accepted before the text reaches the XML parser.
+///
+/// roxmltree parses by recursive descent, so its stack use grows with the nesting depth: ten
+/// thousand levels overflow the main thread's stack and abort the process. A stack overflow is
+/// `abort()`, so `catch_unwind` cannot intercept it and the whole application dies - which is why
+/// the bound has to be applied before parsing, not while walking the result. Around three thousand
+/// levels still parse on the default stack, so this stays clear of the cliff while leaving far more
+/// room than the walks below, which cut off at 64.
+const MAX_XML_DEPTH: usize = 1_024;
+
+/// Element nesting depth of `text`, skipping comments, CDATA, processing instructions, the DOCTYPE
+/// and quoted attribute values. A tag that closes itself (`<field/>`) is one level while it is read
+/// and does not stay open, so a long run of them cannot accumulate into the bound. It cannot
+/// under-count a real element; a stray `<` in text is invalid XML and only makes it over-count,
+/// which errs towards refusing the file.
+fn xml_nesting_depth(text: &str) -> usize {
+    fn find(hay: &[u8], from: usize, needle: &[u8]) -> Option<usize> {
+        (from..hay.len()).find(|&p| hay[p..].starts_with(needle))
+    }
+    let b = text.as_bytes();
+    let (mut i, mut depth, mut max) = (0usize, 0usize, 0usize);
+    while i < b.len() {
+        if b[i] != b'<' {
+            i += 1;
+        } else if b[i..].starts_with(b"<!--") {
+            i = find(b, i + 4, b"-->").map_or(b.len(), |p| p + 3);
+        } else if b[i..].starts_with(b"<![CDATA[") {
+            i = find(b, i + 9, b"]]>").map_or(b.len(), |p| p + 3);
+        } else if b[i..].starts_with(b"<?") {
+            i = find(b, i + 2, b"?>").map_or(b.len(), |p| p + 2);
+        } else if b[i..].starts_with(b"<!") {
+            i = find(b, i + 2, b">").map_or(b.len(), |p| p + 1);
+        } else if b[i..].starts_with(b"</") {
+            depth = depth.saturating_sub(1);
+            i = find(b, i + 2, b">").map_or(b.len(), |p| p + 1);
+        } else {
+            depth += 1;
+            max = max.max(depth);
+            // Skip the rest of the start tag, over any quoted attribute value, and remember its last
+            // significant byte: a tag that ends in `/` closes itself, so it is one level deep while it
+            // is read rather than a level left open.
+            i += 1;
+            let mut quote = 0u8;
+            let mut last = 0u8;
+            while i < b.len() {
+                let c = b[i];
+                if quote != 0 {
+                    if c == quote {
+                        quote = 0;
+                    }
+                } else if c == b'"' || c == b'\'' {
+                    quote = c;
+                } else if c == b'>' {
+                    i += 1;
+                    break;
+                } else if !c.is_ascii_whitespace() {
+                    last = c;
+                }
+                i += 1;
+            }
+            if last == b'/' {
+                depth -= 1;
+            }
+        }
+    }
+    max
+}
+
+fn check_form_xml_depth(text: &str) -> Result<(), DataError> {
+    let depth = xml_nesting_depth(text);
+    if depth > MAX_XML_DEPTH {
+        return Err(DataError::Malformed(format!("the file nests XML elements {depth} deep, past the {MAX_XML_DEPTH} the reader accepts")));
+    }
+    Ok(())
+}
+
+fn parse_form_xml(text: &str) -> Result<roxmltree::Document<'_>, roxmltree::Error> {
+    roxmltree::Document::parse_with_options(text, roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: MAX_NODES })
+}
+
 const SUBTYPES: [(&str, &str); 17] = [
     ("Text", "text"),
     ("FreeText", "freetext"),
@@ -702,7 +788,8 @@ fn place(doc: &mut Document, page_ref: ObjRef, d: Dict) -> Result<ObjRef, DataEr
 }
 
 fn import_xfdf(doc: &mut Document, text: &str) -> Result<Report, DataError> {
-    let xml = roxmltree::Document::parse(text).map_err(|e| DataError::Malformed(e.to_string()))?;
+    check_form_xml_depth(text)?;
+    let xml = parse_form_xml(text).map_err(|e| DataError::Malformed(e.to_string()))?;
     let root = xml.root_element();
     if root.tag_name().name() != "xfdf" {
         return Err(DataError::UnknownFormat);
@@ -754,7 +841,7 @@ fn import_xfdf(doc: &mut Document, text: &str) -> Result<Report, DataError> {
     // Fields.
     if let Some(fields) = root.children().find(|c| c.has_tag_name("fields")) {
         let mut values = Vec::new();
-        walk_xfdf(fields, "", &mut values);
+        walk_xfdf(fields, "", &mut values, 0)?;
         apply_values(doc, &values, &mut report);
     }
     Ok(report)
@@ -787,7 +874,10 @@ fn deep(src: &Document, o: &Object, depth: usize) -> Object {
     }
 }
 
-fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec<String>)>) {
+fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec<String>)>, depth: usize) -> Result<(), DataError> {
+    if depth > MAX_DEPTH {
+        return Err(DataError::Malformed(format!("XFDF fields nest more than {MAX_DEPTH} deep")));
+    }
     for f in node.children().filter(|c| c.has_tag_name("field")) {
         let Some(name) = f.attribute("name") else { continue };
         let full = if prefix.is_empty() { name.to_string() } else { format!("{prefix}.{name}") };
@@ -795,8 +885,9 @@ fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec<Str
         if !vals.is_empty() {
             out.push((full.clone(), vals));
         }
-        walk_xfdf(f, &full, out);
+        walk_xfdf(f, &full, out, depth + 1)?;
     }
+    Ok(())
 }
 
 fn walk_fdf(fdf: &Document, list: &[Object], prefix: &str, out: &mut Vec<(String, Vec<String>)>, depth: usize) {
@@ -857,14 +948,15 @@ pub fn data_values(bytes: &[u8]) -> Result<Vec<(String, Vec<String>)>, DataError
     }
     let text = String::from_utf8_lossy(bytes);
     let t = text.trim_start_matches('\u{feff}').trim_start();
-    let xml = roxmltree::Document::parse(t).map_err(|_| DataError::UnknownFormat)?;
+    check_form_xml_depth(t)?;
+    let xml = parse_form_xml(t).map_err(|_| DataError::UnknownFormat)?;
     let root = xml.root_element();
     if root.tag_name().name() != "xfdf" {
         return Err(DataError::UnknownFormat);
     }
     let mut values = Vec::new();
     if let Some(fields) = root.children().find(|c| c.has_tag_name("fields")) {
-        walk_xfdf(fields, "", &mut values);
+        walk_xfdf(fields, "", &mut values, 0)?;
     }
     Ok(values)
 }
@@ -944,7 +1036,8 @@ fn import_table(doc: &mut Document, text: &str) -> Result<Report, DataError> {
     let mut report = Report::default();
     let t = text.trim_start_matches('\u{feff}');
     let values: Vec<(String, Vec<String>)> = if t.trim_start().starts_with('<') {
-        let xml = roxmltree::Document::parse(t).map_err(|e| DataError::Malformed(e.to_string()))?;
+        check_form_xml_depth(t)?;
+        let xml = parse_form_xml(t).map_err(|e| DataError::Malformed(e.to_string()))?;
         xml.root_element()
             .children()
             .filter(|c| c.is_element())

crates/xfdf/src/tests.rs

@@ -36,6 +36,52 @@ fn blank() -> Document {
     Document::open(Arc::new(write_full(&doc, &SaveOptions::default()).unwrap())).unwrap()
 }
 
+/// A deeply nested field tree is refused with an error. The walk used to recurse once per level
+/// with nothing bounding it, so a small file overflowed the stack - and a stack overflow aborts
+/// the process, which no `catch_unwind` can intercept.
+#[test]
+fn deeply_nested_fields_are_refused_instead_of_overflowing_the_stack() {
+    let mut xml = String::from(r#"<?xml version="1.0"?><xfdf xmlns="http://ns.adobe.com/xfdf/"><fields>"#);
+    for i in 0..10_000 {
+        xml.push_str(&format!(r#"<field name="f{i}">"#));
+    }
+    xml.push_str("<value>v</value>");
+    for _ in 0..10_000 {
+        xml.push_str("</field>");
+    }
+    xml.push_str("</fields></xfdf>");
+    assert!(matches!(data_values(xml.as_bytes()), Err(DataError::Malformed(_))), "ten thousand nested fields must be refused, not walked");
+}
+
+/// Nesting inside the limit still reads, and the dotted field name is built as before.
+#[test]
+fn nested_fields_within_the_limit_still_read() {
+    let mut xml = String::from(r#"<?xml version="1.0"?><xfdf xmlns="http://ns.adobe.com/xfdf/"><fields>"#);
+    for i in 0..16 {
+        xml.push_str(&format!(r#"<field name="f{i}">"#));
+    }
+    xml.push_str("<value>deep</value>");
+    for _ in 0..16 {
+        xml.push_str("</field>");
+    }
+    xml.push_str("</fields></xfdf>");
+    let Ok(values) = data_values(xml.as_bytes()) else { panic!("sixteen levels must still read") };
+    assert_eq!(values.len(), 1);
+    assert_eq!(values[0].0, "f0.f1.f2.f3.f4.f5.f6.f7.f8.f9.f10.f11.f12.f13.f14.f15");
+    assert_eq!(values[0].1, vec!["deep".to_string()]);
+}
+
+/// The scanner counts a tag that closes itself as one level while it is read, not as a level left
+/// open, so a flat file with thousands of them stays shallow and is not refused.
+#[test]
+fn self_closing_elements_do_not_accumulate_in_the_depth() {
+    use crate::xml_nesting_depth as d;
+    assert_eq!(d("<a/>"), 1);
+    assert_eq!(d("<a><b/></a>"), 2);
+    assert_eq!(d(&"<empty/>".repeat(2_000)), 1);
+    assert_eq!(d(&format!("<fields>{}</fields>", r#"<field name="a"/>"#.repeat(2_000))), 2);
+}
+
 /// The blank document with comments and values.
 fn filled() -> Document {
     let mut doc = blank();