diff --git a/crates/svg/src/import.rs b/crates/svg/src/import.rs index b583f3dc03..c17f689922 100644 --- a/crates/svg/src/import.rs +++ b/crates/svg/src/import.rs @@ -137,10 +137,14 @@ pub(crate) fn import_finding( return Err(too_deep()); } let xml = parse(svg)?; + // Before the style cascade of `depth::height` and the prepass, which keeps the declarations of + // each element it reads; measured again below on the text usvg reads when that differs. + let styles = budget::check(&xml, budget::MAX_COPIED_STYLES, depth::MAX_ELEMENT_DEPTH)?; let Some(height) = depth::height(&xml, depth::MAX_ELEMENT_DEPTH) else { return Err(too_deep()); }; let (svg, found) = prepass(svg, &scan.markers, &xml, opts)?; + let rewritten = matches!(svg, Cow::Owned(_)); // The prepass's edits are in the text: the XML is read again only when it made any. let xml = match &svg { Cow::Borrowed(_) => xml, @@ -157,8 +161,9 @@ pub(crate) fn import_finding( let (src, slots) = text::prepare(svg, &xml, text_xml.as_ref(), units.dpi, &mut warnings); // Before usvg copies what ``s show and the style values of each copy: the text usvg reads. let styles = match &src { - Cow::Borrowed(_) => budget::check(&xml, budget::MAX_COPIED_STYLES)?, - Cow::Owned(prepared) => budget::check(&parse(prepared)?, budget::MAX_COPIED_STYLES)?, + Cow::Borrowed(_) if !rewritten => styles, + Cow::Borrowed(_) => budget::check(&xml, budget::MAX_COPIED_STYLES, usize::MAX)?, + Cow::Owned(prepared) => budget::check(&parse(prepared)?, budget::MAX_COPIED_STYLES, usize::MAX)?, }; let files = found.files; files.copied_styles(styles); diff --git a/crates/svg/src/import/budget.rs b/crates/svg/src/import/budget.rs index c2ff01a02f..ec4de17eca 100644 --- a/crates/svg/src/import/budget.rs +++ b/crates/svg/src/import/budget.rs @@ -27,7 +27,7 @@ use std::collections::{HashMap, HashSet}; use usvg::roxmltree; use super::{MAX_NODES, SVG_NS, XNode}; -use crate::SvgError; +use crate::{MAX_ELEMENT_DEPTH, SvgError}; /// Most elements usvg may make of an SVG: [`MAX_NODES`], and below the 1,000,000 elements past /// which usvg stops (its count includes a root of its own). @@ -287,12 +287,14 @@ impl<'a, 'i> Measure<'a, 'i> { } /// The bytes of style values usvg would copy into the elements of `xml`, when that is no more -/// than `bytes_left` and it makes no more than [`MAX_SVG_ELEMENTS`] elements of it. -pub(super) fn check(xml: &roxmltree::Document, bytes_left: u64) -> Result { - match measure(xml, MAX_SVG_ELEMENTS as u64, bytes_left) { +/// than `bytes_left` and it makes no more than [`MAX_SVG_ELEMENTS`] elements of it, nested no +/// deeper than `depth` (counted as `depth::height` counts, which finds at least as much). +pub(super) fn check(xml: &roxmltree::Document, bytes_left: u64, depth: usize) -> Result { + match measure(xml, MAX_SVG_ELEMENTS as u64, bytes_left, depth) { Ok(cost) => Ok(cost.bytes), Err(Past::Elements) => Err(SvgError::TooLarge(MAX_SVG_ELEMENTS)), Err(Past::Bytes) => Err(SvgError::StylesTooLarge((MAX_COPIED_STYLES >> 20) as usize)), + Err(Past::Depth) => Err(SvgError::TooDeep(MAX_ELEMENT_DEPTH)), } } @@ -301,11 +303,13 @@ pub(super) fn check(xml: &roxmltree::Document, bytes_left: u64) -> Result Result { +/// What usvg makes of `xml`, up to `elements` elements and `bytes` bytes of copied styles, its +/// elements and the copies ``s show nested up to `depth` deep. Without recursion; each +/// element is measured once. +fn measure(xml: &roxmltree::Document, elements: u64, bytes: u64, depth: usize) -> Result { let mut m = Measure::new(xml); let mut costs: HashMap = HashMap::new(); // The elements entered and not yet left: one entered again is a loop. @@ -326,7 +330,11 @@ fn measure(xml: &roxmltree::Document, elements: u64, bytes: u64) -> Result depth { + return Err(Past::Depth); } let (parts, bare) = match svg_name(n) { Some("text") => (vec![], None), @@ -376,12 +384,12 @@ mod tests { /// What [`measure`] makes of `text` with no limits, as (elements, bytes). fn cost(text: &str) -> (u64, u64) { let xml = roxmltree::Document::parse(text).unwrap(); - let c = measure(&xml, u64::MAX, u64::MAX).unwrap_or_else(|p| panic!("{p:?}")); + let c = measure(&xml, u64::MAX, u64::MAX, usize::MAX).unwrap_or_else(|p| panic!("{p:?}")); (c.elements, c.bytes) } fn past(text: &str, elements: u64, bytes: u64) -> Option { - measure(&roxmltree::Document::parse(text).unwrap(), elements, bytes).err() + measure(&roxmltree::Document::parse(text).unwrap(), elements, bytes, usize::MAX).err() } /// Groups `g0` to `g{levels}`: `g0` a rectangle styled `style`, each other two ``s of the @@ -445,13 +453,30 @@ mod tests { #[test] fn a_loop_usvg_copies_without_end_is_refused_as_usvg_refuses_it() { let looped = svg(""); - assert_eq!(past(&looped, u64::MAX, u64::MAX), Some(Past::Elements)); + assert_eq!(past(&looped, u64::MAX, u64::MAX), Some(Past::Depth)); assert!(usvg::Tree::from_str(&looped, &usvg::Options::default()).is_err()); // Without the loop, usvg reads it. let open = svg(""); assert!(past(&open, u64::MAX, u64::MAX).is_none() && usvg::Tree::from_str(&open, &usvg::Options::default()).is_ok()); } + /// A chain of ``s nested past the depth is too deep, as `depth::height` finds it, before + /// what the chain makes is added up; one within the depth `depth::height` gives is not. + #[test] + fn copies_nested_past_the_depth_are_too_deep() { + let links: String = (0..3000).map(|i| format!("", i + 1)).collect(); + let deep = svg(&format!("{links}")); + let xml = roxmltree::Document::parse(&deep).unwrap(); + assert_eq!(measure(&xml, u64::MAX, u64::MAX, MAX_ELEMENT_DEPTH).err(), Some(Past::Depth)); + assert!(super::super::depth::height(&xml, MAX_ELEMENT_DEPTH).is_none()); + assert!(matches!(check(&xml, MAX_COPIED_STYLES, MAX_ELEMENT_DEPTH), Err(SvgError::TooDeep(_)))); + for text in [chain(5, ""), svg("")] { + let xml = roxmltree::Document::parse(&text).unwrap(); + let height = super::super::depth::height(&xml, usize::MAX).unwrap_or(0); + assert!(measure(&xml, u64::MAX, u64::MAX, height).is_ok(), "{height} {text}"); + } + } + #[test] fn text_counts_its_runs_and_what_trefs_show() { let (n, bytes) = cost(&svg("abcdxyz")); @@ -459,4 +484,19 @@ mod tests { assert_eq!(n, 1 + 1 + 1 + 2 + 1 + 1 + 1); assert_eq!(bytes, (16 + 2) + (16 + 1) + (16 + 4)); } + + /// A long style sheet rule that matches many elements is refused before the import's own style + /// cascade (which keeps the declarations of each element) reads them, in an SVG and in an SVG + /// image it shows. + #[test] + fn a_long_rule_on_many_elements_is_refused_before_the_cascade_reads_it() { + let groups = svg(&format!("{}", "a".repeat(100_000), "".repeat(1000))); + let image = svg(&format!(r#""#, crate::base64_encode(groups.as_bytes()))); + for text in [groups, image] { + let before = super::super::css::selector_tests(); + let err = super::super::import(&text, &crate::ImportOptions::default(), MAX_NODES).err(); + assert!(matches!(err, Some(SvgError::StylesTooLarge(64))), "{err:?}"); + assert_eq!(super::super::css::selector_tests(), before); + } + } } diff --git a/crates/svg/src/import/css.rs b/crates/svg/src/import/css.rs index fd864a547e..7ddfb1bbd5 100644 --- a/crates/svg/src/import/css.rs +++ b/crates/svg/src/import/css.rs @@ -129,7 +129,7 @@ thread_local! { /// [`SELECTOR_TESTS`] of this thread. #[cfg(test)] -fn selector_tests() -> u64 { +pub(super) fn selector_tests() -> u64 { SELECTOR_TESTS.get() } diff --git a/crates/svg/src/import/files.rs b/crates/svg/src/import/files.rs index 429c39c70f..98c0e4aed4 100644 --- a/crates/svg/src/import/files.rs +++ b/crates/svg/src/import/files.rs @@ -587,22 +587,25 @@ impl Files { return leave_out(&self.too_deep, true); } let xml = super::parse(text).ok()?; - let Some(height) = depth::height(&xml, left) else { return leave_out(&self.too_deep, true) }; - // usvg's parser would overflow on it, as on the SVG holding it (`import`). - super::convolve_matrices_ok(&xml).ok()?; - // What usvg would make of it, measured as the SVG holding it is (`import`), its copied - // styles counted with theirs while usvg reads it, and kept only when it decodes: past the - // limits, the whole SVG is refused. + // What usvg would make of it, measured as the SVG holding it is (`import`), before the + // style cascade of `depth::height`; its copied styles count with theirs while usvg reads + // it, and only when it decodes. Too deep, it is left out; past the other limits, the whole + // SVG is refused. let spent = self.styles.load(Ordering::Relaxed); - match budget::check(&xml, budget::MAX_COPIED_STYLES.saturating_sub(spent)) { - Ok(bytes) => self.styles.store(spent.saturating_add(bytes), Ordering::Relaxed), + let bytes = match budget::check(&xml, budget::MAX_COPIED_STYLES.saturating_sub(spent), left) { + Ok(bytes) => bytes, + Err(SvgError::TooDeep(_)) => return leave_out(&self.too_deep, true), Err(e) => { if let Ok(mut refused) = self.refused.lock() { refused.get_or_insert(e); } return None; } - } + }; + let Some(height) = depth::height(&xml, left) else { return leave_out(&self.too_deep, true) }; + // usvg's parser would overflow on it, as on the SVG holding it (`import`). + super::convolve_matrices_ok(&xml).ok()?; + self.styles.store(spent.saturating_add(bytes), Ordering::Relaxed); self.depth.store(used + height, Ordering::Relaxed); let within = self.within.lock().ok().map(|mut w| w.push(file)).is_some(); // usvg's `Tree::from_data_nested`, except that hrefs to files go to the resolver.