mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-14 19:27:05 -04:00
Never crash: reject SVGs nested too deeply
roxmltree recurses per element and overflowed a 2 MiB stack at a few thousand nested elements (aborting the app). parse() now scans the element depth first and returns Error::Xml beyond 256 levels. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
6708ff55b4
commit
19c1d2ea1d
2 files changed
+72
No files matched your search
@@ -1044,10 +1044,65 @@ pub fn parse_transform(s: &str) -> Affine {
|
||||
m
|
||||
}
|
||||
|
||||
/// The deepest element nesting accepted. The XML parser recurses per level and overflowed a
|
||||
/// 2 MiB thread's stack (aborting the app) at a few thousand levels; real documents stay far
|
||||
/// below this.
|
||||
const MAX_XML_DEPTH: usize = 256;
|
||||
|
||||
/// The deepest element nesting of `text` (a quick scan that skips comments, CDATA, processing
|
||||
/// instructions, declarations and quoted attribute values).
|
||||
fn xml_depth(text: &str) -> usize {
|
||||
let b = text.as_bytes();
|
||||
let (mut i, mut depth, mut max) = (0usize, 0usize, 0usize);
|
||||
let skip_to = |from: usize, end: &[u8]| b.get(from..).and_then(|r| r.windows(end.len()).position(|w| w == end)).map_or(b.len(), |p| from + p + end.len());
|
||||
while i < b.len() {
|
||||
if b[i] != b'<' {
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
let rest = &b[i..];
|
||||
if rest.starts_with(b"<!--") {
|
||||
i = skip_to(i + 4, b"-->");
|
||||
} else if rest.starts_with(b"<![CDATA[") {
|
||||
i = skip_to(i + 9, b"]]>");
|
||||
} else if rest.starts_with(b"<?") {
|
||||
i = skip_to(i + 2, b"?>");
|
||||
} else if rest.starts_with(b"<!") {
|
||||
i = skip_to(i + 2, b">");
|
||||
} else if rest.starts_with(b"</") {
|
||||
depth = depth.saturating_sub(1);
|
||||
i = skip_to(i + 2, b">");
|
||||
} else {
|
||||
// A start tag: find its end outside quotes; `/>` closes it at once.
|
||||
let mut j = i + 1;
|
||||
let mut quote = None;
|
||||
while let Some(&c) = b.get(j) {
|
||||
match quote {
|
||||
Some(q) if c == q => quote = None,
|
||||
Some(_) => {}
|
||||
None if c == b'"' || c == b'\'' => quote = Some(c),
|
||||
None if c == b'>' => break,
|
||||
None => {}
|
||||
}
|
||||
j += 1;
|
||||
}
|
||||
if b.get(j.wrapping_sub(1)) != Some(&b'/') {
|
||||
depth += 1;
|
||||
max = max.max(depth);
|
||||
}
|
||||
i = j + 1;
|
||||
}
|
||||
}
|
||||
max
|
||||
}
|
||||
|
||||
/// Parse an SVG document.
|
||||
pub fn parse(bytes: &[u8]) -> Result<Doc, Error> {
|
||||
let text = String::from_utf8_lossy(bytes);
|
||||
let text = text.trim_start_matches('\u{feff}');
|
||||
if xml_depth(text) > MAX_XML_DEPTH {
|
||||
return Err(Error::Xml(format!("elements nested more than {MAX_XML_DEPTH} levels deep")));
|
||||
}
|
||||
let opts = roxmltree::ParsingOptions { allow_dtd: true, ..Default::default() };
|
||||
let xml = roxmltree::Document::parse_with_options(text, opts).map_err(|e| Error::Xml(e.to_string()))?;
|
||||
let root = xml.root_element();
|
||||
|
||||
@@ -106,3 +106,20 @@ fn transforms() {
|
||||
let p = m * kurbo::Point::new(0.0, 1.0);
|
||||
assert!((p.x - 6.0).abs() < 1e-9 && (p.y - 7.0).abs() < 1e-9);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deep_nesting_errors_instead_of_overflowing_the_stack() {
|
||||
// 100 000 nested groups overflowed the XML parser's stack and aborted the process.
|
||||
let deep = |n: usize| format!("<svg xmlns=\"http://www.w3.org/2000/svg\">{}<rect width=\"5\" height=\"5\"/>{}</svg>", "<g>".repeat(n), "</g>".repeat(n));
|
||||
assert!(matches!(crate::parse(deep(100_000).as_bytes()), Err(crate::Error::Xml(_))));
|
||||
assert!(crate::parse(deep(100).as_bytes()).is_ok());
|
||||
// Comments, CDATA, quoted '>' and self-closing tags don't count.
|
||||
let s = format!(
|
||||
"<svg xmlns=\"http://www.w3.org/2000/svg\"><!-- {} --><![CDATA[{}]]><g title=\"a>b\">{}</g></svg>",
|
||||
"<g>".repeat(1000),
|
||||
"<g>".repeat(1000),
|
||||
"<rect/>".repeat(1000)
|
||||
);
|
||||
assert_eq!(crate::xml_depth(&s), 2);
|
||||
assert!(crate::parse(s.as_bytes()).is_ok());
|
||||
}
|
||||
Reference in new issue
Block a user