SVG import: refuse deeply nested markup instead of overflowing the stack (#1901)
A pre-scan rejects element nesting past 512 levels before the XML parser recurses (a 70 KB file of <g> aborted the process). .svgz is inflated by the importer with a 128 MB cap, so it can be scanned too and gzip bombs are refused; flate2 moves from a dev-dependency to a dependency (already in the tree through usvg). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
sunyd168
GitHub
parent
a342ab8820
commit
0af072e82c
|
|
@ -34,6 +34,8 @@ photocraft-format = { path = "../format" }
|
|||
thiserror = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
blake3 = { version = "1.8", default-features = false, features = ["pure"] }
|
||||
# svg.rs inflates `.svgz` itself (with a size cap) so the XML can be checked before it is parsed.
|
||||
flate2 = { workspace = true }
|
||||
# SVG import (svg.rs): usvg parses, resvg rasterises what the shape model can't hold. Both are
|
||||
# already in the tree through the UI's icon loader; text shaping needs the fonts feature set.
|
||||
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
|
||||
|
|
|
|||
|
|
@ -32,6 +32,12 @@ const MAX_RASTER_PIXELS: u64 = 24_000_000;
|
|||
const MAX_NODES: usize = 2_000;
|
||||
/// Nesting past which a subtree is rasterised instead of walked (bounded recursion).
|
||||
const MAX_RECURSION: usize = 200;
|
||||
/// Deepest element nesting accepted. The XML parser recurses once per level, so a file nested
|
||||
/// a few thousand deep (70 KB of `<g>`) overflows a 2 MB thread stack, an abort no guard can
|
||||
/// catch; real drawings stay under a hundred levels.
|
||||
pub const MAX_XML_DEPTH: usize = 512;
|
||||
/// Largest `.svgz` accepted once inflated (a 1.5 MB gzip can hold 1.5 GB of whitespace).
|
||||
pub const MAX_INFLATED_BYTES: u64 = 128 << 20;
|
||||
|
||||
/// Does this look like an SVG document (plain XML; `.svgz` is recognised by its extension)?
|
||||
pub fn is_svg(bytes: &[u8]) -> bool {
|
||||
|
|
@ -65,9 +71,8 @@ fn fonts() -> Arc<usvg::fontdb::Database> {
|
|||
/// `~/.ssh/...`) into the document. Embedded SVG images get no `<image>` resolution at all
|
||||
/// (`usvg` already blanks it for sub-SVGs). The font database (and its one-off scan of the system
|
||||
/// fonts) is only loaded when the drawing may contain text.
|
||||
fn options(bytes: &[u8]) -> usvg::Options<'static> {
|
||||
// `.svgz` is gzip: it can't be scanned before usvg inflates it, so assume it has text.
|
||||
let has_text = bytes.starts_with(&[0x1f, 0x8b]) || bytes.windows(4).any(|w| w == b"text");
|
||||
fn options(xml: &[u8]) -> usvg::Options<'static> {
|
||||
let has_text = xml.windows(4).any(|w| w == b"text");
|
||||
usvg::Options {
|
||||
resources_dir: None,
|
||||
image_href_resolver: usvg::ImageHrefResolver { resolve_data: usvg::ImageHrefResolver::default_data_resolver(), resolve_string: Box::new(|_, _| None) },
|
||||
|
|
@ -76,9 +81,87 @@ fn options(bytes: &[u8]) -> usvg::Options<'static> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Parses a drawing, `.svgz` included. The XML is inflated and checked here, before the parser
|
||||
/// sees it: deeper nesting than [`MAX_XML_DEPTH`] is an error, not a stack overflow.
|
||||
fn parse(bytes: &[u8]) -> Result<usvg::Tree, IoError> {
|
||||
let opt = options(bytes);
|
||||
usvg::Tree::from_data(bytes, &opt).map_err(|e| IoError::Svg(e.to_string()))
|
||||
let xml = inflate(bytes)?;
|
||||
if nesting_depth(&xml, MAX_XML_DEPTH) > MAX_XML_DEPTH {
|
||||
return Err(IoError::Svg(format!("elements are nested more than {MAX_XML_DEPTH} levels deep")));
|
||||
}
|
||||
let opt = options(&xml);
|
||||
usvg::Tree::from_data(&xml, &opt).map_err(|e| IoError::Svg(e.to_string()))
|
||||
}
|
||||
|
||||
/// The plain XML of `bytes`: itself, or the inflated content of a gzip `.svgz`, refused past
|
||||
/// [`MAX_INFLATED_BYTES`] (a decompression bomb) or when the stream is corrupt.
|
||||
fn inflate(bytes: &[u8]) -> Result<std::borrow::Cow<'_, [u8]>, IoError> {
|
||||
use std::io::Read;
|
||||
if !bytes.starts_with(&[0x1f, 0x8b]) {
|
||||
return Ok(std::borrow::Cow::Borrowed(bytes));
|
||||
}
|
||||
let mut out = Vec::new();
|
||||
let mut reader = flate2::read::MultiGzDecoder::new(bytes).take(MAX_INFLATED_BYTES.saturating_add(1));
|
||||
reader.read_to_end(&mut out).map_err(|e| IoError::Svg(format!("the compressed SVG couldn't be inflated: {e}")))?;
|
||||
if u64::try_from(out.len()).unwrap_or(u64::MAX) > MAX_INFLATED_BYTES {
|
||||
return Err(IoError::Svg(format!("the compressed SVG inflates past {} MB", MAX_INFLATED_BYTES >> 20)));
|
||||
}
|
||||
Ok(std::borrow::Cow::Owned(out))
|
||||
}
|
||||
|
||||
/// The deepest element nesting in `xml`, stopping as soon as it passes `limit` (the result is
|
||||
/// then `limit + 1`). Comments, CDATA sections, processing instructions and the DOCTYPE add
|
||||
/// nothing; quoted attribute values may hold `>` or `/>`. Malformed markup is only counted
|
||||
/// loosely: the parser rejects it afterwards.
|
||||
fn nesting_depth(xml: &[u8], limit: usize) -> usize {
|
||||
let after = |from: usize, pat: &[u8]| -> usize {
|
||||
xml.get(from..).and_then(|s| s.windows(pat.len()).position(|w| w == pat)).map_or(xml.len(), |p| from.saturating_add(p).saturating_add(pat.len()))
|
||||
};
|
||||
// The byte after a tag's closing `>` (outside quotes) and whether the tag ended in `/>`.
|
||||
let tag_end = |from: usize| -> (usize, bool) {
|
||||
let (mut quote, mut last) = (None, b' ');
|
||||
for (k, &b) in xml.iter().enumerate().skip(from) {
|
||||
match quote {
|
||||
Some(q) if b == q => quote = None,
|
||||
Some(_) => {}
|
||||
None if b == b'"' || b == b'\'' => quote = Some(b),
|
||||
None if b == b'>' => return (k.saturating_add(1), last == b'/'),
|
||||
None => {
|
||||
if !b.is_ascii_whitespace() {
|
||||
last = b;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
(xml.len(), false)
|
||||
};
|
||||
let (mut depth, mut deepest, mut i) = (0usize, 0usize, 0usize);
|
||||
while let Some(p) = xml.get(i..).and_then(|s| s.iter().position(|&b| b == b'<')) {
|
||||
let at = i.saturating_add(p);
|
||||
let rest = xml.get(at..).unwrap_or_default();
|
||||
i = if rest.starts_with(b"<!--") {
|
||||
after(at.saturating_add(4), b"-->")
|
||||
} else if rest.starts_with(b"<![CDATA[") {
|
||||
after(at.saturating_add(9), b"]]>")
|
||||
} else if rest.starts_with(b"<?") {
|
||||
after(at.saturating_add(2), b"?>")
|
||||
} else if rest.starts_with(b"<!") {
|
||||
tag_end(at.saturating_add(2)).0
|
||||
} else if rest.starts_with(b"</") {
|
||||
depth = depth.saturating_sub(1);
|
||||
tag_end(at.saturating_add(2)).0
|
||||
} else {
|
||||
let (end, self_closing) = tag_end(at.saturating_add(1));
|
||||
if !self_closing {
|
||||
depth = depth.saturating_add(1);
|
||||
deepest = deepest.max(depth);
|
||||
if deepest > limit {
|
||||
return deepest;
|
||||
}
|
||||
}
|
||||
end
|
||||
};
|
||||
}
|
||||
deepest
|
||||
}
|
||||
|
||||
/// The document canvas for a drawing: its size in px, scaled down to [`MAX_SIDE`] when needed.
|
||||
|
|
|
|||
|
|
@ -215,6 +215,74 @@ fn size_comes_from_the_viewbox_and_huge_drawings_shrink_to_fit() {
|
|||
assert!(px(d, w / 4, h / 2)[3] < 0.01 && px(d, 3 * w / 4, h / 2)[3] > 0.99);
|
||||
}
|
||||
|
||||
fn nested(depth: usize) -> String {
|
||||
format!("<svg {NS} width=\"8\" height=\"8\">{}<rect width=\"8\" height=\"8\"/>{}</svg>", "<g>".repeat(depth), "</g>".repeat(depth))
|
||||
}
|
||||
|
||||
fn gzip(bytes: &[u8]) -> Vec<u8> {
|
||||
use std::io::Write;
|
||||
let mut e = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::fast());
|
||||
e.write_all(bytes).unwrap();
|
||||
e.finish().unwrap()
|
||||
}
|
||||
|
||||
/// 8 000 nested groups (70 KB) overflowed the XML parser's recursion on a 2 MB thread stack, an
|
||||
/// abort nothing can catch. It is an error now, found before the parser runs; the thread here
|
||||
/// has a small stack so a regression aborts the test instead of passing by luck.
|
||||
#[test]
|
||||
fn deeply_nested_markup_is_an_error_not_a_stack_overflow() {
|
||||
let deep = nested(8_000);
|
||||
let gz = gzip(deep.as_bytes());
|
||||
std::thread::Builder::new()
|
||||
.stack_size(512 * 1024)
|
||||
.spawn(move || {
|
||||
for (name, bytes) in [("deep.svg", deep.as_bytes()), ("deep.svgz", gz.as_slice())] {
|
||||
let e = import(name, bytes).err().unwrap_or_else(|| panic!("{name} opened"));
|
||||
assert!(matches!(&e, IoError::Svg(m) if m.contains("nested") && m.contains(&svg::MAX_XML_DEPTH.to_string())), "{name}: {e}");
|
||||
}
|
||||
})
|
||||
.unwrap()
|
||||
.join()
|
||||
.unwrap();
|
||||
// Just inside the limit still opens, the empty groups folded away.
|
||||
let r = open(&nested(svg::MAX_XML_DEPTH - 1));
|
||||
assert_eq!(r.document.layers.len(), 1);
|
||||
}
|
||||
|
||||
/// The depth check reads the markup, not the parser's tree: it must not count `<` in comments,
|
||||
/// CDATA, processing instructions or the DOCTYPE, nor end a tag at a `>` inside quotes.
|
||||
#[test]
|
||||
fn the_depth_check_skips_comments_cdata_doctype_and_quoted_markup() {
|
||||
let svg = format!(
|
||||
"<?xml version=\"1.0\"?><!-- {g} --><!DOCTYPE svg PUBLIC \"-//W3C//DTD SVG 1.1//EN\" \"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd\">\
|
||||
<svg {NS} width=\"8\" height=\"8\"><desc><![CDATA[ {g} ]]></desc><?pi {g} ?>\
|
||||
<g id='a>b' data-x=\"c/>d\"><rect width=\"8\" height=\"8\" fill=\"#ff0000\"/></g><!-- {g} --></svg>",
|
||||
g = "<g>".repeat(svg::MAX_XML_DEPTH + 10)
|
||||
);
|
||||
let r = open(&svg);
|
||||
assert_eq!(r.document.layers.len(), 1);
|
||||
assert!(close(px(&r.document, 4, 4), [1.0, 0.0, 0.0, 1.0]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn svgz_is_inflated_with_a_size_cap() {
|
||||
let plain = format!("<svg {NS} width=\"8\" height=\"8\"><rect width=\"8\" height=\"8\" fill=\"#0000ff\"/></svg>");
|
||||
let r = import("drawing.svgz", &gzip(plain.as_bytes())).unwrap();
|
||||
assert_eq!(r.document.layers.len(), 1);
|
||||
assert!(close(px(&r.document, 4, 4), [0.0, 0.0, 1.0, 1.0]));
|
||||
// A bomb: a few hundred KB of gzip holding more than the cap of whitespace.
|
||||
let mut bomb = format!("<svg {NS} width=\"8\" height=\"8\">").into_bytes();
|
||||
bomb.resize(usize::try_from(svg::MAX_INFLATED_BYTES).unwrap() + 1024, b' ');
|
||||
bomb.extend_from_slice(b"</svg>");
|
||||
let gz = gzip(&bomb);
|
||||
assert!(gz.len() < 2 << 20, "{} bytes compressed", gz.len());
|
||||
let e = import("bomb.svgz", &gz).err().unwrap_or_else(|| panic!("the bomb opened"));
|
||||
assert!(matches!(&e, IoError::Svg(m) if m.contains("inflates past")), "{e}");
|
||||
// Corrupt gzip is an error too.
|
||||
let e = import("bad.svgz", &[0x1f, 0x8b, 0x08, 0x00, 0xff, 0xff]).err().unwrap_or_else(|| panic!("corrupt gzip opened"));
|
||||
assert!(matches!(e, IoError::Svg(_)), "{e}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sniffed_without_an_extension_and_refused_when_malformed() {
|
||||
let svg = format!("<?xml version=\"1.0\"?>\n<svg {NS} width=\"8\" height=\"8\"><rect width=\"8\" height=\"8\"/></svg>");
|
||||
|
|
|
|||
Loading…
Reference in New Issue