forked from 1b-agent/opencadstudio-upstream
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.
This commit is contained in:
Karim Jerbi committed
1 parent
9bbee20c6c
commit
21c4d6ab41
1 file changed
+282
-6
@@ -52,6 +52,144 @@ fn numbers(text: &str) -> Vec<f64> {
|
||||
.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 `</…`
|
||||
let mut quote = 0u8; // inside a quoted attribute value
|
||||
let mut mark = 0u8; // 1 comment, 2 cdata, 3 PI, 4 doctype/decl
|
||||
let mut brackets = 0usize; // doctype internal subset `[ … ]`
|
||||
while i < b.len() {
|
||||
match mark {
|
||||
1 => {
|
||||
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 (`<a/>`, `<a />`) 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"<!--") {
|
||||
mark = 1;
|
||||
i += 4;
|
||||
} else if rest.starts_with(b"<![CDATA[") {
|
||||
mark = 2;
|
||||
i += 9;
|
||||
} else if rest.starts_with(b"<?") {
|
||||
mark = 3;
|
||||
i += 2;
|
||||
} else if rest.starts_with(b"<!") {
|
||||
mark = 4;
|
||||
brackets = 0;
|
||||
i += 2;
|
||||
} else if rest.starts_with(b"</") {
|
||||
tag = true;
|
||||
closing = true;
|
||||
i += 2;
|
||||
} else {
|
||||
tag = true;
|
||||
closing = false;
|
||||
i += 1;
|
||||
}
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// `roxmltree::Document::parse` behind the depth ceiling (`MAX_XML_DEPTH`).
|
||||
fn parse_xml(text: &str) -> Option<roxmltree::Document<'_>> {
|
||||
if !xml_depth_within(text, MAX_XML_DEPTH) {
|
||||
log::warn!(
|
||||
"XML nests deeper than {MAX_XML_DEPTH} levels ({} bytes); document rejected",
|
||||
text.len()
|
||||
);
|
||||
return None;
|
||||
}
|
||||
roxmltree::Document::parse(text).ok()
|
||||
}
|
||||
|
||||
/// Every sheet the package plots, in plot order.
|
||||
fn sheets(files: &HashMap<String, Vec<u8>>) -> Vec<SheetEntry> {
|
||||
let mut out = Vec::new();
|
||||
@@ -60,7 +198,7 @@ fn sheets(files: &HashMap<String, Vec<u8>>) -> Vec<SheetEntry> {
|
||||
continue;
|
||||
}
|
||||
let Ok(text) = std::str::from_utf8(data) else { continue };
|
||||
let Ok(doc) = roxmltree::Document::parse(text.trim_start_matches('\u{feff}')) else { continue };
|
||||
let Some(doc) = parse_xml(text.trim_start_matches('\u{feff}')) else { continue };
|
||||
let page = doc.root_element();
|
||||
if page.tag_name().name() != "Page" {
|
||||
continue;
|
||||
@@ -689,7 +827,7 @@ mod xps {
|
||||
let mut units = Units::identity();
|
||||
let mut view_log = None;
|
||||
if let Some(w2x) = w2x.and_then(|b| std::str::from_utf8(b).ok()) {
|
||||
if let Ok(doc) = roxmltree::Document::parse(w2x.trim_start_matches('\u{feff}')) {
|
||||
if let Some(doc) = parse_xml(w2x.trim_start_matches('\u{feff}')) {
|
||||
for node in doc.descendants() {
|
||||
match node.tag_name().name() {
|
||||
"Units" => {
|
||||
@@ -709,7 +847,7 @@ mod xps {
|
||||
}
|
||||
}
|
||||
let text = std::str::from_utf8(page).ok()?;
|
||||
let doc = roxmltree::Document::parse(text.trim_start_matches('\u{feff}')).ok()?;
|
||||
let doc = parse_xml(text.trim_start_matches('\u{feff}'))?;
|
||||
// Page units (1/96 inch, y down) → graphics units: paper millimetres
|
||||
// with y up, less the paper offset, over the graphics scale.
|
||||
let (scale, offset) = entry.paper;
|
||||
@@ -763,7 +901,7 @@ mod xps {
|
||||
if stroke.is_some() || fill.is_some() {
|
||||
paths.push(Path { subpaths, stroke, fill });
|
||||
}
|
||||
});
|
||||
}, 0);
|
||||
let view = view_log.map(|v: [f64; 4]| {
|
||||
let a = units.model([v[0], v[1]]);
|
||||
let b = units.model([v[2], v[3]]);
|
||||
@@ -772,15 +910,29 @@ mod xps {
|
||||
Some((paths, view, texts))
|
||||
}
|
||||
|
||||
fn walk(node: roxmltree::Node, m: Affine, f: &mut impl FnMut(roxmltree::Node, Affine)) {
|
||||
/// Deepest `<Canvas>` nesting `walk` will follow. It recurses once per
|
||||
/// level on file-controlled XML (roxmltree parses iteratively, so only
|
||||
/// this walk bounds depth): ~10k levels overflowed the stack. Real
|
||||
/// pages nest single digits; 64 matches the DGN CFB walker's cap.
|
||||
const MAX_CANVAS_DEPTH: usize = 64;
|
||||
|
||||
fn walk(
|
||||
node: roxmltree::Node,
|
||||
m: Affine,
|
||||
f: &mut impl FnMut(roxmltree::Node, Affine),
|
||||
depth: usize,
|
||||
) {
|
||||
for child in node.children().filter(|n| n.is_element()) {
|
||||
match child.tag_name().name() {
|
||||
"Canvas" => {
|
||||
if depth >= MAX_CANVAS_DEPTH {
|
||||
continue;
|
||||
}
|
||||
let m2 = match attr(child, "RenderTransform").and_then(parse_affine) {
|
||||
Some(local) => mul(m, local),
|
||||
None => m,
|
||||
};
|
||||
walk(child, m2, f);
|
||||
walk(child, m2, f, depth + 1);
|
||||
}
|
||||
"Path" | "Glyphs" => f(child, m),
|
||||
_ => {}
|
||||
@@ -958,3 +1110,127 @@ mod xps {
|
||||
pieces
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod xps_depth_tests {
|
||||
use super::{xps, SheetEntry};
|
||||
|
||||
fn entry() -> SheetEntry {
|
||||
SheetEntry {
|
||||
name: "Page1".into(),
|
||||
order: 0,
|
||||
graphics: String::new(),
|
||||
paper: ([1.0, 1.0], [0.0, 0.0]),
|
||||
paper_height_mm: 297.0,
|
||||
w2x: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// The report's 100 000-deep `<Canvas>` chain. roxmltree's tokenizer
|
||||
/// itself recurses per level (`parse_element` ↔ `parse_content`), so
|
||||
/// unguarded `Document::parse` died with STATUS_STACK_OVERFLOW before
|
||||
/// `walk` ever ran; `walk` was separately unbounded (both capped now).
|
||||
/// The page is rejected instead of parsed.
|
||||
#[test]
|
||||
fn deeply_nested_canvases_do_not_overflow_the_stack() {
|
||||
const DEPTH: usize = 100_000;
|
||||
let mut xml = String::with_capacity(DEPTH * 17 + 128);
|
||||
xml.push_str(
|
||||
r#"<FixedPage xmlns="http://schemas.microsoft.com/xps/2005/08" Width="100" Height="100">"#,
|
||||
);
|
||||
for _ in 0..DEPTH {
|
||||
xml.push_str("<Canvas>");
|
||||
}
|
||||
for _ in 0..DEPTH {
|
||||
xml.push_str("</Canvas>");
|
||||
}
|
||||
xml.push_str("</FixedPage>");
|
||||
|
||||
assert!(xps::read(xml.as_bytes(), None, &entry(), &|_| None).is_none());
|
||||
}
|
||||
|
||||
/// Ordinary nesting still walks through (and still emits geometry).
|
||||
#[test]
|
||||
fn modestly_nested_canvases_still_walk() {
|
||||
let xml = r##"<FixedPage xmlns="http://schemas.microsoft.com/xps/2005/08" Width="100" Height="100">
|
||||
<Canvas><Canvas><Path Data="M 10 10 L 50 50" Stroke="#FF0000"/></Canvas></Canvas>
|
||||
</FixedPage>"##;
|
||||
let (paths, _view, _texts) =
|
||||
xps::read(xml.as_bytes(), None, &entry(), &|_| None).expect("page parses");
|
||||
assert_eq!(paths.len(), 1);
|
||||
}
|
||||
|
||||
/// Boundary at MAX_XML_DEPTH, and markup that must not count:
|
||||
/// quoted attribute values (with `</`+`<` inside), comments, CDATA,
|
||||
/// processing instructions, and self-closing tags.
|
||||
#[test]
|
||||
fn depth_scanner_counts_real_elements_only() {
|
||||
let nest = |n: usize| -> String {
|
||||
let mut s = "<r>".repeat(n);
|
||||
s.push_str(&"</r>".repeat(n));
|
||||
s
|
||||
};
|
||||
assert!(super::xml_depth_within(&nest(super::MAX_XML_DEPTH), super::MAX_XML_DEPTH));
|
||||
assert!(!super::xml_depth_within(&nest(super::MAX_XML_DEPTH + 1), super::MAX_XML_DEPTH));
|
||||
|
||||
// 1000 hostile-looking chunks, actual depth 1 each.
|
||||
let mut s = String::from("<r>");
|
||||
for _ in 0..1000 {
|
||||
s.push_str(r#"<n t="</n><n><!-- --> <![CDATA[<b></b>]]>"/>
|
||||
"#);
|
||||
}
|
||||
s.push_str(&"</r>".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(&"<a/>".repeat(5000), super::MAX_XML_DEPTH));
|
||||
assert!(super::xml_depth_within(
|
||||
&format!("<?xml version='1.0'?><a><!-- {} --></a>", "<b></b>".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("<Units>");
|
||||
for _ in 0..100_000 {
|
||||
deep.push_str("<Canvas>");
|
||||
}
|
||||
for _ in 0..100_000 {
|
||||
deep.push_str("</Canvas>");
|
||||
}
|
||||
deep.push_str("</Units>");
|
||||
let xml = r##"<FixedPage xmlns="http://schemas.microsoft.com/xps/2005/08" Width="100" Height="100">
|
||||
<Path Data="M 10 10 L 50 50" Stroke="#FF0000"/>
|
||||
</FixedPage>"##;
|
||||
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("<Page>");
|
||||
for _ in 0..100_000 {
|
||||
deep.push_str("<Canvas>");
|
||||
}
|
||||
for _ in 0..100_000 {
|
||||
deep.push_str("</Canvas>");
|
||||
}
|
||||
deep.push_str("</Page>");
|
||||
let mut files = std::collections::HashMap::new();
|
||||
files.insert(
|
||||
"3D/descriptor.xml".to_string(),
|
||||
deep.into_bytes(),
|
||||
);
|
||||
assert!(super::sheets(&files).is_empty());
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user