Importers: XML element nesting and DTD group nesting are bounded (#712)

The DOCX and IDML element-tree builders accepted any depth, and the
tree is then walked and dropped recursively; a DTD content model's
groups recursed without a budget. Each parser now refuses nesting past
a fixed budget with an error, before any recursive consumer runs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011K1peyrh5MdZ6ttHkVq8Gh
This commit is contained in:
wstock
2026-10-15 07:13:51 +00:00
co-authored by Claude Fable 5.1
parent a930276fd7
commit 02bc1e38aa
3 changed files with 59 additions and 5 deletions
+22 -5
View File
@@ -101,12 +101,19 @@ fn tokens(s: &str) -> Vec<String> {
out
}
/// Parse a content particle from `t[*i..]`.
fn particle(t: &[String], i: &mut usize) -> Result<Cp, String> {
/// 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<Cp, String> {
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<Cp, String> {
}
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<Model, String> {
}
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 {
<!ELEMENT figure EMPTY>
<!ATTLIST figure href CDATA #REQUIRED alt CDATA "a > 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!("<!ELEMENT a {}b{}>", "(".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();
+18
View File
@@ -194,6 +194,11 @@ pub fn document_with_pi(root: &El, pi: &str) -> Vec<u8> {
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<El, String> {
let mut r = quick_xml::Reader::from_reader(bytes);
@@ -205,6 +210,9 @@ pub fn parse(bytes: &[u8]) -> Result<El, String> {
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<El, String> {
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!("{}{}", "<A>".repeat(n), "</A>".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 & <c>"));
+19
View File
@@ -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<El, ImportError> {
let mut r = quick_xml::Reader::from_str(xml);
let mut stack: Vec<El> = 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!("{}{}", "<w:p>".repeat(n), "</w:p>".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<u8> {
docx_parts(&[("word/document.xml", document), ("word/styles.xml", styles), ("word/footnotes.xml", footnotes)])
}