diff --git a/crates/engine/src/dtd.rs b/crates/engine/src/dtd.rs index 7f752efaa..9b64e65c1 100644 --- a/crates/engine/src/dtd.rs +++ b/crates/engine/src/dtd.rs @@ -101,12 +101,19 @@ fn tokens(s: &str) -> Vec { out } -/// Parse a content particle from `t[*i..]`. -fn particle(t: &[String], i: &mut usize) -> Result { +/// How deep content-model groups may nest: real DTDs stay in single digits, and each group is a +/// recursive call, so an unbounded `((((…` is refused rather than run into the stack (#712). +const MAX_GROUP_DEPTH: usize = 64; + +/// Parse a content particle from `t[*i..]`; `depth` is how many groups enclose it. +fn particle(t: &[String], i: &mut usize, depth: usize) -> Result { let base = match t.get(*i).map(String::as_str) { Some("(") => { + if depth >= MAX_GROUP_DEPTH { + return Err(format!("content-model groups nested deeper than {MAX_GROUP_DEPTH} levels")); + } *i += 1; - let mut items = vec![particle(t, i)?]; + let mut items = vec![particle(t, i, depth + 1)?]; let mut sep = None; loop { match t.get(*i).map(String::as_str) { @@ -120,7 +127,7 @@ fn particle(t: &[String], i: &mut usize) -> Result { } sep = Some(if s == "," { "," } else { "|" }); *i += 1; - items.push(particle(t, i)?); + items.push(particle(t, i, depth + 1)?); } other => return Err(format!("unexpected `{}` in a content model", other.unwrap_or("end"))), } @@ -155,7 +162,7 @@ fn parse_model(s: &str) -> Result { } let t = tokens(s); let mut i = 0; - let cp = particle(&t, &mut i)?; + let cp = particle(&t, &mut i, 0)?; if i != t.len() { return Err(format!("unexpected `{}` after the content model", t[i])); } @@ -422,6 +429,16 @@ mod tests { b" kind (photo|chart) "photo">"##; + /// Issue #712: a content model nested past the group budget is an error, not a stack overflow. + #[test] + fn content_model_groups_past_the_budget_are_refused() { + let model = |n: usize| format!("", "(".repeat(n), ")".repeat(n)); + assert!(parse(&model(MAX_GROUP_DEPTH)).is_ok(), "the budget itself parses"); + let err = parse(&model(MAX_GROUP_DEPTH + 1)).expect_err("one level more is refused"); + assert!(err.contains("nested deeper"), "{err}"); + assert!(parse(&model(100_000)).is_err()); + } + #[test] fn content_models_and_required_attributes() { let dtd = parse(DTD).unwrap(); diff --git a/crates/idml/src/xml.rs b/crates/idml/src/xml.rs index 97aab239b..b3f1ef0d0 100644 --- a/crates/idml/src/xml.rs +++ b/crates/idml/src/xml.rs @@ -194,6 +194,11 @@ pub fn document_with_pi(root: &El, pi: &str) -> Vec { s.into_bytes() } +/// How deep elements may nest. InDesign's packages stay under a few dozen levels; the tree is +/// walked and dropped recursively afterwards, so unbounded nesting is refused here, not left to +/// run into the stack (#712). +pub const MAX_DEPTH: usize = 256; + /// Parse a document into its root element. pub fn parse(bytes: &[u8]) -> Result { let mut r = quick_xml::Reader::from_reader(bytes); @@ -205,6 +210,9 @@ pub fn parse(bytes: &[u8]) -> Result { let ev = r.read_event_into(&mut buf).map_err(|e| format!("XML error at {}: {e}", pos(&r)))?; match ev { Event::Start(e) => { + if stack.len() > MAX_DEPTH { + return Err(format!("XML elements nested deeper than {MAX_DEPTH} levels at {}", pos(&r))); + } let el = start_el(&e)?; stack.push(el); } @@ -296,6 +304,16 @@ fn start_el(e: &quick_xml::events::BytesStart) -> Result { mod tests { use super::*; + /// Issue #712: nesting past the budget is an error from the parser, not a stack overflow later. + #[test] + fn nesting_past_the_budget_is_refused() { + let nested = |n: usize| format!("{}{}", "".repeat(n), "".repeat(n)).into_bytes(); + assert!(parse(&nested(MAX_DEPTH)).is_ok(), "the budget itself parses"); + let err = parse(&nested(MAX_DEPTH + 1)).expect_err("one level more is refused"); + assert!(err.contains("nested deeper"), "{err}"); + assert!(parse(&nested(100_000)).is_err()); + } + #[test] fn roundtrip_text_pi_and_entities() { let root = El::new("Root").attr("A", "x\"<&\t").child(El::new("Content").text("a\tb & ")); diff --git a/crates/textimport/src/docx.rs b/crates/textimport/src/docx.rs index 20101f824..3f89566d3 100644 --- a/crates/textimport/src/docx.rs +++ b/crates/textimport/src/docx.rs @@ -57,12 +57,20 @@ fn local(n: &[u8]) -> String { s.rsplit(':').next().unwrap_or(&s).to_string() } +/// How deep elements may nest. Word's own XML stays well under a hundred levels; the tree is read +/// recursively afterwards (`El::text`, the block walk) and dropped recursively, so a document +/// nested without bound must be refused here, not run into the stack (#712). +pub(crate) const MAX_DEPTH: usize = 256; + pub(crate) fn parse(xml: &str) -> Result { let mut r = quick_xml::Reader::from_str(xml); let mut stack: Vec = vec![El::default()]; loop { match r.read_event().map_err(|e| ImportError::Corrupt(e.to_string()))? { Event::Start(e) => { + if stack.len() > MAX_DEPTH { + return Err(ImportError::Corrupt(format!("XML elements nested deeper than {MAX_DEPTH} levels"))); + } let el = El { name: local(e.name().as_ref()), attrs: e @@ -543,6 +551,17 @@ mod tests { use super::*; use std::io::Write; + /// Issue #712: nesting past the budget is an error from the parser, not a stack overflow later. + #[test] + fn nesting_past_the_budget_is_refused() { + let nested = |n: usize| format!("{}{}", "".repeat(n), "".repeat(n)); + let ok = parse(&nested(MAX_DEPTH)).expect("the budget itself parses"); + assert_eq!(ok.name, "p"); + let err = parse(&nested(MAX_DEPTH + 1)).expect_err("one level more is refused"); + assert!(err.to_string().contains("nested deeper"), "{err}"); + assert!(parse(&nested(100_000)).is_err()); + } + pub(crate) fn docx(document: &str, styles: &str, footnotes: &str) -> Vec { docx_parts(&[("word/document.xml", document), ("word/styles.xml", styles), ("word/footnotes.xml", footnotes)]) }