From 35f5faa2e673133a81686e88e6a6de9aba1e95f1 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 24 Oct 2026 11:25:43 +0200 Subject: [PATCH] Check SVG nesting depth after DTD entity expansion (#1177) Fixes #1081 Co-authored-by: Claude Opus 5.5 --- crates/svg/src/import/budget.rs | 2 +- crates/svg/src/import/hostile_tests.rs | 15 ++++++++++ crates/svg/src/import/mod.rs | 38 +++++++++++++++++++++++++- 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/crates/svg/src/import/budget.rs b/crates/svg/src/import/budget.rs index e7ae3fc6..75a5e673 100644 --- a/crates/svg/src/import/budget.rs +++ b/crates/svg/src/import/budget.rs @@ -74,7 +74,7 @@ fn embedded(used: &Used, limits: &Limits, data: &[u8], opts: &Options) -> Option let left = limits.embedded.saturating_sub(used.bytes.load(Ordering::Relaxed)); let plain = inflate(data, left).ok()?; used.bytes.fetch_add(plain.len(), Ordering::Relaxed); - if super::nesting(&plain, super::MAX_NESTING) > super::MAX_NESTING { + if super::too_deep(&plain) { return None; } used.depth.fetch_add(1, Ordering::Relaxed); diff --git a/crates/svg/src/import/hostile_tests.rs b/crates/svg/src/import/hostile_tests.rs index bb750286..0af1297d 100644 --- a/crates/svg/src/import/hostile_tests.rs +++ b/crates/svg/src/import/hostile_tests.rs @@ -166,3 +166,18 @@ fn nesting_is_measured_before_parsing() { let p = d.first_page().unwrap(); assert!(import_svg(&mut d, &svg(&deep), p, 0.0, 0.0, "x").is_err()); } + +#[test] +fn entity_expansion_cannot_bypass_the_nesting_limit() { + // An internal-DTD entity whose value nests 300 deep, referenced inside a body nesting 300 deep: + // the parsed tree is ~600 deep although the raw bytes never are. + let inner = format!("{}{}", "".repeat(300), "".repeat(300)); + let body = format!("{}&deep;{}", "".repeat(300), "".repeat(300)); + let data = format!( + "]>{body}" + ); + let mut d = Document::new("t"); + let p = d.first_page().unwrap(); + let err = import_svg(&mut d, data.as_bytes(), p, 0.0, 0.0, "x").err().map(|e| e.to_string()).unwrap_or_default(); + assert!(err.contains("nested more than 512 deep"), "{err:?}"); +} diff --git a/crates/svg/src/import/mod.rs b/crates/svg/src/import/mod.rs index 3aed363a..dc936260 100644 --- a/crates/svg/src/import/mod.rs +++ b/crates/svg/src/import/mod.rs @@ -672,6 +672,42 @@ pub(crate) fn nesting(data: &[u8], limit: usize) -> usize { max } +/// Whether `data` nests elements more than [`MAX_NESTING`] deep: the byte scan first, then the +/// parsed tree (walked without recursion), which also counts what DTD entity references expand to. +pub(crate) fn too_deep(data: &[u8]) -> bool { + if nesting(data, MAX_NESTING) > MAX_NESTING { + return true; + } + let Ok(text) = std::str::from_utf8(data) else { return false }; + let opt = usvg::roxmltree::ParsingOptions { allow_dtd: true, ..Default::default() }; + let Ok(xml) = usvg::roxmltree::Document::parse_with_options(text, opt) else { return false }; + // Depth-first by sibling and parent links; the root is depth 0, each child one deeper. + let (mut node, mut depth) = (xml.root(), 0usize); + loop { + if let Some(child) = node.first_child() { + node = child; + depth += 1; + if depth > MAX_NESTING { + return true; + } + continue; + } + loop { + if let Some(next) = node.next_sibling() { + node = next; + break; + } + match node.parent() { + Some(up) if depth > 0 => { + node = up; + depth -= 1; + } + _ => return false, + } + } + } +} + fn options(data: &[u8], limits: budget::Limits) -> usvg::Options<'static> { // Only data URIs, within budgets: an opened or pasted SVG never pulls other files off the disk // and never expands without limit. @@ -728,7 +764,7 @@ fn import_within( ) -> Result { // SVGZ is unpacked here, within a budget, so every check below sees the real document. let data = budget::document(data, &limits).map_err(ImportError)?; - if nesting(&data, MAX_NESTING) > MAX_NESTING { + if too_deep(&data) { return Err(ImportError(format!("elements nested more than {MAX_NESTING} deep"))); } let tree = usvg::Tree::from_data(&data, &options(&data, limits)).map_err(|e| ImportError(e.to_string()))?;