fix(xfa): AcroForm field walks read each field once and bound what they queue

crates/xfa/src/pdf.rs

@@ -90,7 +90,7 @@ struct Signed {
     appearance: Option<Object>,
 }
 
-/// Most fields looked at for signatures.
+/// Most fields looked at for signatures, counted as they are queued.
 const MAX_FIELDS_SEARCHED: usize = 20_000;
 /// Deepest field nesting followed.
 const MAX_FIELD_DEPTH: usize = 32;
@@ -105,21 +105,29 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap<String, Signed>,
     let mut names = HashSet::new();
     let Some(acro) = catalog.get(b"AcroForm").map(|a| doc.resolve(a)) else { return (out, names) };
     let Some(fields) = acro.as_dict().and_then(|a| a.get(b"Fields")).map(|f| doc.resolve(f)) else { return (out, names) };
+    // Counted as queued, not as visited: a field listing one kid many times (or itself) would
+    // otherwise queue them all on every visit, far past the limit, before the count stopped it.
     let mut stack: Vec<(Object, String, bool, usize)> =
-        fields.as_array().into_iter().flatten().map(|f| (f.clone(), String::new(), false, 0)).collect();
-    let mut seen = 0usize;
+        fields.as_array().into_iter().flatten().take(MAX_FIELDS_SEARCHED).map(|f| (f.clone(), String::new(), false, 0)).collect();
+    let mut queued = stack.len();
+    // And each field is read once, however many times it is listed.
+    let mut seen = HashSet::new();
     while let Some((field, prefix, parent_is_sig, depth)) = stack.pop() {
-        seen += 1;
-        if seen > MAX_FIELDS_SEARCHED {
-            break;
+        if let Object::Ref(r) = field
+            && !seen.insert(r)
+        {
+            continue;
         }
         let field = doc.resolve(&field);
         let Some(d) = field.as_dict() else { continue };
         if d.contains(NODE_KEY) {
             // Generated to name a signature field: only what it holds is looked at.
             if depth < MAX_FIELD_DEPTH {
-                let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default();
-                stack.extend(kids.into_iter().map(|k| (k, String::new(), false, depth + 1)));
+                let kids = d.get(b"Kids").map(|k| doc.resolve(k));
+                let kids = kids.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default();
+                let room = MAX_FIELDS_SEARCHED.saturating_sub(queued);
+                queued += kids.len().min(room);
+                stack.extend(kids.iter().take(room).map(|k| (k.clone(), String::new(), false, depth + 1)));
             }
             continue;
         }
@@ -145,14 +153,17 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap<String, Signed>,
             Some(ft) => ft == b"Sig",
             None => parent_is_sig,
         };
-        let kids: Vec<Object> = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default();
+        let kids = d.get(b"Kids").map(|k| doc.resolve(k));
+        let kids = kids.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default();
         if is_sig && let Some(value) = d.get(b"V").filter(|v| doc.resolve(v).as_dict().is_some()) {
             // The appearance is on the field when it is its own widget, or on its one widget.
             let appearance = d.get(b"AP").cloned().or_else(|| kids.iter().find_map(|k| doc.resolve(k).as_dict().and_then(|k| k.get(b"AP").cloned())));
             out.entry(name.clone()).or_insert(Signed { value: value.clone(), appearance });
         }
         if depth < MAX_FIELD_DEPTH {
-            stack.extend(kids.into_iter().map(|k| (k, name.clone(), is_sig, depth + 1)));
+            let room = MAX_FIELDS_SEARCHED.saturating_sub(queued);
+            queued += kids.len().min(room);
+            stack.extend(kids.iter().take(room).map(|k| (k.clone(), name.clone(), is_sig, depth + 1)));
         }
     }
     (out, names)
@@ -961,7 +972,49 @@ pub fn pdf_rect(r: Rect, page_height: f64) -> [f64; 4] {
 
 #[cfg(test)]
 mod tests {
-    use super::parse_hex32;
+    use pdfcraft_cos::{Dict, Document, Object, PdfString};
+
+    use super::{MAX_FIELDS_SEARCHED, existing_fields, parse_hex32};
+
+    /// The fields `existing_fields` finds under an AcroForm listing `fields`.
+    fn names_found(doc: &Document, fields: Vec<Object>) -> Vec<String> {
+        let mut acro = Dict::new();
+        acro.set(b"Fields".to_vec(), Object::Array(fields));
+        let mut catalog = Dict::new();
+        catalog.set(b"AcroForm".to_vec(), Object::Dict(acro));
+        let (signed, names) = existing_fields(doc, &catalog);
+        assert!(signed.is_empty());
+        let mut names: Vec<String> = names.into_iter().collect();
+        names.sort();
+        names
+    }
+
+    fn field(name: &str, kids: Object) -> Object {
+        let mut d = Dict::new();
+        d.set(b"T".to_vec(), Object::String(PdfString::literal(name)));
+        d.set(b"Kids".to_vec(), kids);
+        Object::Dict(d)
+    }
+
+    #[test]
+    fn fields_listed_many_times_queue_a_bounded_number_of_kids() {
+        // A field lists itself 5,000 times: queuing every kid on each of 20,000 visits would
+        // hold a hundred million entries. It is read once.
+        let mut doc = Document::new_empty();
+        let r = doc.add(Object::Null);
+        doc.set(r, field("a", Object::Array(vec![Object::Ref(r); 5_000])));
+        assert_eq!(names_found(&doc, vec![Object::Ref(r)]), ["a"]);
+
+        // 5,000 fields share one /Kids array listing each of them 200 times. Read once each,
+        // the fields down one branch still queue the whole array at every level: 32 million
+        // entries, unless the queue itself is bounded.
+        let mut doc = Document::new_empty();
+        let kids = doc.add(Object::Null);
+        let fields: Vec<Object> = (0..5_000).map(|i| Object::Ref(doc.add(field(&format!("f{i}"), Object::Ref(kids))))).collect();
+        doc.set(kids, Object::Array(fields.iter().cycle().take(1_000_000).cloned().collect()));
+        let names = names_found(&doc, fields[..1].to_vec());
+        assert!(names.contains(&"f0".to_string()) && names.len() <= MAX_FIELDS_SEARCHED, "{} names", names.len());
+    }
 
     #[test]
     fn parse_hex32_rejects_non_hex_digits() {

crates/xfa/src/script.rs

@@ -2,7 +2,7 @@
 //! events a template declares, the presence overrides scripts make (kept in the PDF so they
 //! survive undo and reopening), and laying the form out again after scripts changed it.
 
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, HashMap, HashSet};
 
 use pdfcraft_cos::{Dict, Document, Object, PdfString};
 
@@ -15,6 +15,8 @@ pub const OVERRIDES_KEY: &[u8] = b"PCXfaOverrides";
 
 const MAX_DEPTH: usize = 64;
 const MAX_EVENTS: usize = 20_000;
+/// Most `/Kids` entries [`fields_by_som`] follows in all, fields already read included.
+const MAX_FIELD_STEPS: usize = 1 << 20;
 /// Most bytes of script the events hold together: each instance of a repeated subform holds its
 /// own copy, so a long script repeated many times stops here (as much as the XFA packets hold).
 pub(crate) const MAX_EVENT_SCRIPT_BYTES: usize = 64 << 20;
@@ -543,8 +545,17 @@ pub fn apply_overrides(tpl: &Template, ov: &Overrides) -> Template {
 
 /// The generated fields of the AcroForm by SOM path: `som → (field name, is a radio group)`.
 pub fn fields_by_som(doc: &Document) -> HashMap<String, String> {
-    fn walk(doc: &Document, r: pdfcraft_cos::ObjRef, prefix: &str, out: &mut HashMap<String, String>, depth: usize) {
-        if depth > MAX_DEPTH || out.len() > 100_000 {
+    // Each field is read once: a field listed twice in `/Kids` (or among its own kids) would
+    // otherwise be walked again for every path to it, doubling the work at each level. `steps`
+    // counts every entry followed, so fields sharing one long `/Kids` array stay bounded too.
+    struct Walk {
+        out: HashMap<String, String>,
+        seen: HashSet<pdfcraft_cos::ObjRef>,
+        steps: usize,
+    }
+    fn walk(doc: &Document, r: pdfcraft_cos::ObjRef, prefix: &str, w: &mut Walk, depth: usize) {
+        w.steps += 1;
+        if depth > MAX_DEPTH || w.out.len() > 100_000 || w.steps > MAX_FIELD_STEPS || !w.seen.insert(r) {
             return;
         }
         let o = doc.get(r);
@@ -558,21 +569,21 @@ pub fn fields_by_som(doc: &Document) -> HashMap<String, String> {
         if let Some(som) = d.get(crate::pdf::SOM_KEY).map(|s| doc.resolve(s)).and_then(|s| s.as_string().map(|s| s.to_text()))
             && t.is_some()
         {
-            out.insert(som, name.clone());
+            w.out.insert(som, name.clone());
         }
         if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) {
             for k in kids.iter().filter_map(Object::as_ref) {
-                walk(doc, k, &name, out, depth + 1);
+                walk(doc, k, &name, w, depth + 1);
             }
         }
     }
-    let mut out = HashMap::new();
-    let Some((_, acro)) = acroform(doc) else { return out };
+    let Some((_, acro)) = acroform(doc) else { return HashMap::new() };
     let fields = acro.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default();
+    let mut w = Walk { out: HashMap::new(), seen: HashSet::new(), steps: 0 };
     for f in fields.iter().filter_map(Object::as_ref) {
-        walk(doc, f, "", &mut out, 0);
+        walk(doc, f, "", &mut w, 0);
     }
-    out
+    w.out
 }
 
 /// Lay the form out again from its template, data and overrides, replacing the pages and the

crates/xfa/src/tests.rs

@@ -4089,3 +4089,30 @@ fn clear_password_values_clears_static_xfa_password_fields() {
     let packets = read_packets(&doc).unwrap().unwrap();
     assert!(!packets.xdp.contains("hunter2"), "{}", packets.xdp);
 }
+
+#[test]
+fn fields_by_som_reads_a_field_listed_many_times_once() {
+    // A field lists itself twice and its one widget twice: walked once per path to it, that is
+    // 2^64 visits at the depth limit.
+    let s = |t: &str| Object::String(pdfcraft_cos::PdfString::literal(t));
+    let mut doc = Document::new_empty();
+    let mut widget = Dict::new();
+    widget.set(b"T".to_vec(), s("name"));
+    widget.set(crate::pdf::SOM_KEY.to_vec(), s("form1[0].name[0]"));
+    let widget = Object::Ref(doc.add(Object::Dict(widget)));
+    let parent = doc.add(Object::Null);
+    let mut d = Dict::new();
+    d.set(b"T".to_vec(), s("form1"));
+    d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(parent), widget.clone(), Object::Ref(parent), widget]));
+    doc.set(parent, Object::Dict(d));
+    let mut acro = Dict::new();
+    acro.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(parent)]));
+    let mut cat = Dict::new();
+    cat.set(b"Type".to_vec(), Object::Name(b"Catalog".to_vec()));
+    cat.set(b"AcroForm".to_vec(), Object::Dict(acro));
+    let cat = doc.add(Object::Dict(cat));
+    doc.trailer_mut().set(b"Root".to_vec(), Object::Ref(cat));
+    let by_som = fields_by_som(&doc);
+    assert_eq!(by_som.len(), 1, "{by_som:?}");
+    assert_eq!(by_som.get("form1[0].name[0]").map(String::as_str), Some("form1.name"));
+}