From 21c4d6ab41e090e6ee400cbdb277787e3294c46f Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Tue, 29 Sep 2026 11:00:44 +0100 Subject: [PATCH] fix(dwf): bound XML element depth before roxmltree and Canvas walk Bug: Loading a DWFx package recursed twice on untrusted input: 1. roxmltree's tokenizer (parse_element <-> parse_content) recurses once per element nesting level (present in versions 0.20 and 0.21) - a page or descriptor nested about 100k deep overflows the stack inside Document::parse, before any application code runs; 2. the XPS walk recursed once per nested Canvas with no limit - about 10k levels overflowed the stack. Either one aborts the process during ATTACH. Reproduce: 1. Build a DWFx (zip) whose .fpage / descriptor.xml nests elements (e.g. Canvas) about 100,000 deep. 2. ATTACH it. 3. Before: STATUS_STACK_OVERFLOW (0xc00000fd) - process death; a test calling only roxmltree::Document::parse on the same bytes reproduces it, proving the dependency's tokenizer dies first. 4. After: the file is rejected before parse (a log::warn records "XML nests deeper than 256 levels ... document rejected"), or - for Canvas nesting between 65 and 256 - the page renders with the too-deep subtree skipped while siblings and the rest of the page stay. Fix: A linear depth scan (xml_depth_within, skipping comments, CDATA, PIs, doctype subsets and quoted attribute values so their angle brackets never count; it may over-reject malformed markup but must never under-count) gates all three Document::parse sites at 256 levels - real pages nest single digits - and walk caps Canvas nesting at 64 (continue skips that subtree, siblings unaffected), matching the compound-file walker's cap. Rejections are logged so a rejected depth is distinguishable from a corrupt parse. --- src/scene/model/underlay_vector/dwf.rs | 288 ++++++++++++++++++++++++- 1 file changed, 282 insertions(+), 6 deletions(-) diff --git a/src/scene/model/underlay_vector/dwf.rs b/src/scene/model/underlay_vector/dwf.rs index e654c0721..32bf54b39 100644 --- a/src/scene/model/underlay_vector/dwf.rs +++ b/src/scene/model/underlay_vector/dwf.rs @@ -52,6 +52,144 @@ fn numbers(text: &str) -> Vec { .collect() } +/// Ceiling on element nesting depth before roxmltree sees the text. Its +/// tokenizer recurses once per level (`parse_element` ↔ `parse_content`, +/// present in 0.20 and 0.21), so a 100 000-deep fpage overflows the stack +/// *inside* `Document::parse`. Real pages and descriptors nest single +/// digits; 256 keeps the frames well under the thread stack budget. +const MAX_XML_DEPTH: usize = 256; + +/// True when no element nests deeper than `max`. A linear byte scan that +/// skips comments, CDATA, PIs, doctype subsets and quoted attribute values +/// so their `<`/`>` never count. Defensive only: malformed markup may +/// overcount and reject the file, but must never undercount. +fn xml_depth_within(text: &str, max: usize) -> bool { + let b = text.as_bytes(); + let mut i = 0usize; + let mut depth = 0usize; + let mut tag = false; // inside `< … >` + let mut closing = false; // the current tag is ` { + if b[i..].starts_with(b"-->") { + mark = 0; + i += 3; + } else { + i += 1; + } + continue; + } + 2 => { + if b[i..].starts_with(b"]]>") { + mark = 0; + i += 3; + } else { + i += 1; + } + continue; + } + 3 => { + if b[i..].starts_with(b"?>") { + mark = 0; + i += 2; + } else { + i += 1; + } + continue; + } + 4 => { + match b[i] { + b'[' => brackets += 1, + b']' => brackets = brackets.saturating_sub(1), + b'>' if brackets == 0 => mark = 0, + _ => {} + } + i += 1; + continue; + } + _ => {} + } + let c = b[i]; + if quote != 0 { + if c == quote { + quote = 0; + } + i += 1; + continue; + } + if tag { + match c { + b'"' | b'\'' => quote = c, + b'>' => { + tag = false; + if closing { + depth = depth.saturating_sub(1); + } else { + // Self-closing (``, ``) adds no depth. + let mut j = i; + while j > 0 && b[j - 1].is_ascii_whitespace() { + j -= 1; + } + if !(j > 0 && b[j - 1] == b'/') { + depth += 1; + if depth > max { + return false; + } + } + } + } + _ => {} + } + i += 1; + continue; + } + if c != b'<' { + i += 1; + continue; + } + let rest = &b[i..]; + if rest.starts_with(b" ]]>"/> +"#); + } + s.push_str(&"".repeat(1000)); + assert!(super::xml_depth_within(&s, super::MAX_XML_DEPTH)); + + // Self-closing tags add no depth; only real opens do. + assert!(super::xml_depth_within(&"".repeat(5000), super::MAX_XML_DEPTH)); + assert!(super::xml_depth_within( + &format!("", "".repeat(1000)), + super::MAX_XML_DEPTH + )); + } + + /// The w2x units resource goes through the same guarded parse; a deep + /// one is skipped while the page itself still renders. + #[test] + fn deep_w2x_is_skipped_without_hurting_the_page() { + let mut deep = String::from(""); + for _ in 0..100_000 { + deep.push_str(""); + } + for _ in 0..100_000 { + deep.push_str(""); + } + deep.push_str(""); + let xml = r##" + + "##; + let (paths, _view, _texts) = xps::read( + xml.as_bytes(), + Some(deep.as_bytes()), + &entry(), + &|_| None, + ) + .expect("page parses"); + assert_eq!(paths.len(), 1); + } + + /// A hostile descriptor.xml is dropped instead of crashing `sheets`. + #[test] + fn deep_descriptor_produces_no_sheets() { + let mut deep = String::from(""); + for _ in 0..100_000 { + deep.push_str(""); + } + for _ in 0..100_000 { + deep.push_str(""); + } + deep.push_str(""); + let mut files = std::collections::HashMap::new(); + files.insert( + "3D/descriptor.xml".to_string(), + deep.into_bytes(), + ); + assert!(super::sheets(&files).is_empty()); + } +}