diff --git a/crates/xfa/src/data.rs b/crates/xfa/src/data.rs index ac80d419b2..9c0f51dd1a 100644 --- a/crates/xfa/src/data.rs +++ b/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 { + 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) -> 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. diff --git a/crates/xfa/src/lib.rs b/crates/xfa/src/lib.rs index a1758ab479..f9fd29d3b0 100644 --- a/crates/xfa/src/lib.rs +++ b/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 (``) 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 { + (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"").map_or(b.len(), |p| p + 3); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 3); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 2); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 1); + } else if b[i..].starts_with(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, diff --git a/crates/xfa/src/parse.rs b/crates/xfa/src/parse.rs index d3111f375c..2de0bb8390 100644 --- a/crates/xfa/src/parse.rs +++ b/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), 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(); diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 1ef3216e2d..91fcf7789d 100644 --- a/crates/xfa/src/tests.rs +++ b/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#""); + 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(""), 3); + assert_eq!(d(r#""#), 1); + assert_eq!(d("]]>"), 1); + assert_eq!(d(r#""#), 2); + assert_eq!(d(""), 1); + assert_eq!(d(""), 2); + assert_eq!(d(&"".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#""); + 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#""); + 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!( diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index e0dfdf9c18..7bec9c5f41 100644 --- a/crates/xfdf/src/lib.rs +++ b/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 (``) 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 { + (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"").map_or(b.len(), |p| p + 3); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 3); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 2); + } else if b[i..].starts_with(b"").map_or(b.len(), |p| p + 1); + } else if b[i..].starts_with(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::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 Result { - 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 { // 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)>) { +fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec)>, 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)>, depth: usize) { @@ -857,14 +948,15 @@ pub fn data_values(bytes: &[u8]) -> Result)>, 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 { let mut report = Report::default(); let t = text.trim_start_matches('\u{feff}'); let values: Vec<(String, Vec)> = 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()) diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 3e5635a9a8..0f32cced8a 100644 --- a/crates/xfdf/src/tests.rs +++ b/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#""#); + for i in 0..10_000 { + xml.push_str(&format!(r#""#)); + } + xml.push_str("v"); + for _ in 0..10_000 { + xml.push_str(""); + } + xml.push_str(""); + 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#""#); + for i in 0..16 { + xml.push_str(&format!(r#""#)); + } + xml.push_str("deep"); + for _ in 0..16 { + xml.push_str(""); + } + xml.push_str(""); + 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(""), 1); + assert_eq!(d(""), 2); + assert_eq!(d(&"".repeat(2_000)), 1); + assert_eq!(d(&format!("{}", r#""#.repeat(2_000))), 2); +} + /// The blank document with comments and values. fn filled() -> Document { let mut doc = blank();