diff --git a/crates/xfa/src/pdf.rs b/crates/xfa/src/pdf.rs index 60f4835ee8..9117bcc37c 100644 --- a/crates/xfa/src/pdf.rs +++ b/crates/xfa/src/pdf.rs @@ -94,6 +94,8 @@ struct Signed { const MAX_FIELDS_SEARCHED: usize = 20_000; /// Deepest field nesting followed. const MAX_FIELD_DEPTH: usize = 32; +/// Most `/Fields` and `/Kids` entries read while looking, repeats included. +const MAX_KIDS_READ: usize = 1 << 20; /// The fields the document came with: the full name of each, and the signed signature fields by /// SOM path. Signing applications name a dynamic form's fields by their SOM path @@ -105,29 +107,16 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap, 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().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() { - if let Object::Ref(r) = field - && !seen.insert(r) - { - continue; - } - let field = doc.resolve(&field); + let mut walk = FieldQueue::default(); + walk.queue(fields.as_array().map(Vec::as_slice).unwrap_or_default(), "", false, 0); + while let Some((field, prefix, parent_is_sig, depth)) = walk.stack.pop() { + let field = doc.get(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)); - 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))); + walk.queue(kids.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default(), "", false, depth + 1); } continue; } @@ -161,14 +150,42 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap, out.entry(name.clone()).or_insert(Signed { value: value.clone(), appearance }); } if depth < MAX_FIELD_DEPTH { - 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))); + walk.queue(kids, &name, is_sig, depth + 1); } } (out, names) } +/// The fields [`existing_fields`] still has to look at: each with its parent's full name, +/// whether it inherits a signature type, and its depth. +#[derive(Default)] +struct FieldQueue { + stack: Vec<(ObjRef, String, bool, usize)>, + seen: HashSet, + read: usize, +} + +impl FieldQueue { + /// Queue the fields `kids` lists, each the first time it is listed. Fields and kids are + /// indirect references (ISO 32000-2 ยง12.7.3.1); anything else is skipped. A field listed + /// again, by this list or another, takes no room, and at most [`MAX_FIELDS_SEARCHED`] are + /// ever queued: a field listing one kid many times, or fields sharing one long `/Kids`, would + /// otherwise queue it all on every visit. Reading the repeats costs too, so at most + /// [`MAX_KIDS_READ`] entries are read in all. + fn queue(&mut self, kids: &[Object], prefix: &str, is_sig: bool, depth: usize) { + for k in kids { + self.read += 1; + if self.seen.len() >= MAX_FIELDS_SEARCHED || self.read > MAX_KIDS_READ { + return; + } + let Some(r) = k.as_ref() else { continue }; + if self.seen.insert(r) { + self.stack.push((r, prefix.to_string(), is_sig, depth)); + } + } + } +} + impl Emitter<'_> { fn da(face: &Face) -> Vec { format!("/{} {} Tf {} rg", face.resource_name(), n(face.size), rgb(face.color)).into_bytes() @@ -1016,6 +1033,27 @@ mod tests { assert!(names.contains(&"f0".to_string()) && names.len() <= MAX_FIELDS_SEARCHED, "{} names", names.len()); } + #[test] + fn a_field_listed_many_times_leaves_room_for_the_fields_under_it() { + // The AcroForm lists one field 19,999 times. The copies take no room, so its kids are + // still queued, and the signed one among them is found. + let mut doc = Document::new_empty(); + let plain = doc.add(field("plain", Object::Array(Vec::new()))); + let mut sig = Dict::new(); + sig.set(b"T".to_vec(), Object::String(PdfString::literal("sig"))); + sig.set(b"FT".to_vec(), Object::Name(b"Sig".to_vec())); + sig.set(b"V".to_vec(), Object::Dict(Dict::new())); + let sig = doc.add(Object::Dict(sig)); + let parent = doc.add(field("c", Object::Array(vec![Object::Ref(plain), Object::Ref(sig)]))); + let mut acro = Dict::new(); + acro.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(parent); MAX_FIELDS_SEARCHED - 1])); + let mut catalog = Dict::new(); + catalog.set(b"AcroForm".to_vec(), Object::Dict(acro)); + let (signed, names) = existing_fields(&doc, &catalog); + assert!(signed.contains_key("c.sig"), "{:?}", signed.keys().collect::>()); + assert!(names.contains("c.plain")); + } + #[test] fn parse_hex32_rejects_non_hex_digits() { for prefix in ["+f", "-f", " f"] { diff --git a/crates/xfa/src/script.rs b/crates/xfa/src/script.rs index 84d3c1888b..3e70849bee 100644 --- a/crates/xfa/src/script.rs +++ b/crates/xfa/src/script.rs @@ -547,19 +547,23 @@ pub fn apply_overrides(tpl: &Template, ov: &Overrides) -> Template { pub fn fields_by_som(doc: &Document) -> HashMap { // 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. + // counts every entry followed, so fields sharing one long `/Kids` array stay bounded too; + // `walk` returns false once it is spent, and every loop then stops. struct Walk { out: HashMap, seen: HashSet, steps: usize, } - fn walk(doc: &Document, r: pdfcraft_cos::ObjRef, prefix: &str, w: &mut Walk, depth: usize) { + fn walk(doc: &Document, r: pdfcraft_cos::ObjRef, prefix: &str, w: &mut Walk, depth: usize) -> bool { w.steps += 1; - if depth > MAX_DEPTH || w.out.len() > 100_000 || w.steps > MAX_FIELD_STEPS || !w.seen.insert(r) { - return; + if w.steps > MAX_FIELD_STEPS || w.out.len() > 100_000 { + return false; + } + if depth > MAX_DEPTH || !w.seen.insert(r) { + return true; } let o = doc.get(r); - let Some(d) = o.as_dict() else { return }; + let Some(d) = o.as_dict() else { return true }; let t = d.get(b"T").map(|t| doc.resolve(t)).and_then(|t| t.as_string().map(|s| s.to_text())); let name = match (&t, prefix.is_empty()) { (Some(t), true) => t.clone(), @@ -571,18 +575,14 @@ pub fn fields_by_som(doc: &Document) -> HashMap { { 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, w, depth + 1); - } - } + // Read in place: a copy of the array at every level would cost its length each time. + let kids = d.get(b"Kids").map(|k| doc.resolve(k)); + kids.as_deref().and_then(Object::as_array).into_iter().flatten().filter_map(Object::as_ref).all(|k| walk(doc, k, &name, w, depth + 1)) } 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 fields = acro.get(b"Fields").map(|f| doc.resolve(f)); 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 w, 0); - } + let _ = fields.as_deref().and_then(Object::as_array).into_iter().flatten().filter_map(Object::as_ref).all(|f| walk(doc, f, "", &mut w, 0)); w.out }