diff --git a/crates/io/Cargo.toml b/crates/io/Cargo.toml index 763297d1..ffe99e4d 100644 --- a/crates/io/Cargo.toml +++ b/crates/io/Cargo.toml @@ -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] diff --git a/crates/io/src/svg.rs b/crates/io/src/svg.rs index 58fc996b..693b725a 100644 --- a/crates/io/src/svg.rs +++ b/crates/io/src/svg.rs @@ -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 ``) 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 { /// `~/.ssh/...`) into the document. Embedded SVG images get no `` 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 { - 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, 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"") + } else if rest.starts_with(b"") + } else if rest.starts_with(b"") + } else if rest.starts_with(b" limit { + return deepest; + } + } + end + }; + } + deepest } /// The document canvas for a drawing: its size in px, scaled down to [`MAX_SIDE`] when needed. diff --git a/crates/io/tests/svg.rs b/crates/io/tests/svg.rs index 8eab3933..1784b8a5 100644 --- a/crates/io/tests/svg.rs +++ b/crates/io/tests/svg.rs @@ -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!("{}{}", "".repeat(depth), "".repeat(depth)) +} + +fn gzip(bytes: &[u8]) -> Vec { + 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!( + "\ + {g} \ + d\">", + 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!(""); + 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!("").into_bytes(); + bomb.resize(usize::try_from(svg::MAX_INFLATED_BYTES).unwrap() + 1024, b' '); + bomb.extend_from_slice(b""); + 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!("\n");