From ef4e13dc2d8cc65d570cb2c13d1ae34648ddf317 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 22 Oct 2026 06:22:10 -0400 Subject: [PATCH] security(io): stop double decompression of gzip-compressed SVG The importer bounded and inflated the top-level .svgz once, then handed the result to usvg::Tree::from_data, which re-checks the gzip magic and decompresses again with no byte limit of its own if the content happens to start with a nested gzip stream. The same default resolver decompresses embedded image/svg+xml data URLs (SVGZ-in-data-URL) the same unbounded way. Parse the already-bounded top-level text with Tree::from_str, which never decompresses. Wrap the image-href resolver so a gzip-compressed embedded SVG is inflated through our bounded inflate() first; the already-plain bytes handed to the default resolver no longer start with the gzip magic, so it parses them directly instead of decompressing a second time. Finding: a6444163-e510-4925-ae63-727658b6582e (CWE-409) --- crates/io/src/svg.rs | 39 ++++++++++++++++++++++++++++++++- crates/io/tests/it/svg.rs | 46 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/crates/io/src/svg.rs b/crates/io/src/svg.rs index d656f58103..6bc256d4e0 100644 --- a/crates/io/src/svg.rs +++ b/crates/io/src/svg.rs @@ -194,6 +194,18 @@ fn options(fontdb: Arc) -> usvg::Options<'static> { return None; } } + // An embedded `image/svg+xml` value can itself be gzip-compressed (SVGZ-in-data-URL). + // The default resolver's sub-SVG loader checks for the gzip magic and decompresses + // again with no byte limit of its own. Apply our bounded inflate first: if the data is + // gzip-compressed, hand the default resolver the already-decompressed (and + // size-checked) bytes, which no longer start with the gzip magic, so it parses them + // directly instead of decompressing a second time. + if (mime == "image/svg+xml" || mime == "text/plain") && data.starts_with(&[0x1f, 0x8b]) { + return match inflate(&data) { + Ok(xml) if !xml.starts_with(&[0x1f, 0x8b]) => default_resolver("image/svg+xml", std::sync::Arc::new(xml.into_owned()), opts), + _ => None, + }; + } default_resolver(mime, data, opts) }); usvg::Options { @@ -231,7 +243,11 @@ fn parse_with(bytes: &[u8], fonts: fn() -> Arc) -> Resul let fontdb = if has_text { fonts() } else { Arc::new(usvg::fontdb::Database::new()) }; let fontless_text = has_text && fontdb.faces().next().is_none(); let opt = options(fontdb); - let tree = usvg::Tree::from_data(&xml, &opt).map_err(|e| IoError::Svg(e.to_string()))?; + // Parse the already-bounded-and-decoded text with from_str, not from_data: from_data + // re-checks the gzip magic and would decompress a second time without our size limit if + // the inflated content happened to start with a nested gzip stream. + let text = std::str::from_utf8(&xml).map_err(|_| IoError::Svg("the SVG is not valid UTF-8".into()))?; + let tree = usvg::Tree::from_str(text, &opt).map_err(|e| IoError::Svg(e.to_string()))?; Ok(Parsed { tree, fontless_text }) } @@ -1136,4 +1152,25 @@ mod tests { assert!(resolve("image/webp", Arc::new(bad_webp.clone()), &opt).is_none()); assert!(resolve("text/plain", Arc::new(bad_webp), &opt).is_none()); } + + #[test] + fn resolve_data_rejects_nested_gzip_and_handles_text_plain_svgz() { + use std::io::Write; + fn gz(bytes: &[u8]) -> Vec { + let mut e = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::fast()); + e.write_all(bytes).unwrap(); + e.finish().unwrap() + } + let opt = options(Arc::new(usvg::fontdb::Database::new())); + let resolve = &opt.image_href_resolver.resolve_data; + let svg = b""; + let once = gz(svg); + assert!(resolve("image/svg+xml", Arc::new(once.clone()), &opt).is_some()); + assert!(resolve("text/plain", Arc::new(once.clone()), &opt).is_some()); + + // Doubly gzip-compressed payload must be rejected rather than decompressed a second time by usvg. + let twice = gz(&once); + assert!(resolve("image/svg+xml", Arc::new(twice.clone()), &opt).is_none()); + assert!(resolve("text/plain", Arc::new(twice), &opt).is_none()); + } } diff --git a/crates/io/tests/it/svg.rs b/crates/io/tests/it/svg.rs index 517190bcac..c71bb0ae4f 100644 --- a/crates/io/tests/it/svg.rs +++ b/crates/io/tests/it/svg.rs @@ -282,6 +282,52 @@ fn gzip(bytes: &[u8]) -> Vec { e.finish().unwrap() } +/// A `data:` URL of `bytes` under `mime`, base64-encoded. +fn base64_data_url(mime: &str, bytes: &[u8]) -> String { + const T: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut out = format!("data:{mime};base64,"); + for chunk in bytes.chunks(3) { + let n = chunk.len(); + let v = (u32::from(chunk[0]) << 16) | (u32::from(*chunk.get(1).unwrap_or(&0)) << 8) | u32::from(*chunk.get(2).unwrap_or(&0)); + for i in 0..4 { + if i <= n { + out.push(T[((v >> (18 - 6 * i)) & 63) as usize] as char); + } else { + out.push('='); + } + } + } + out +} + +#[test] +fn gzip_compressed_embedded_svg_decompresses_once_with_a_size_cap() { + // A small gzip-compressed SVG embedded via data URL (SVGZ-in-data-URL) must still import. + let inner = format!(""); + let url = base64_data_url("image/svg+xml", &gzip(inner.as_bytes())); + let r = open(&format!("")); + assert!( + r.document.layers.iter().any(|l| matches!(l.content, LayerContent::Raster(_))), + "valid embedded gzip SVG must decode into a raster layer: {:?}", + r.document.layers + ); + + // A gzip-compressed embedded SVG whose decompressed size exceeds the cap must be rejected + // by our bounded inflate, not silently decompressed again without a limit by usvg. + 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 url = base64_data_url("image/svg+xml", &gz); + let start = std::time::Instant::now(); + let r = open(&format!("")); + assert!(start.elapsed() < std::time::Duration::from_secs(5), "took {:?}", start.elapsed()); + // The embedded bomb is rejected; the outer drawing may still open (e.g. with a missing + // image), but must not have decoded the bomb into a raster layer. + assert!(r.document.layers.iter().all(|l| !matches!(l.content, LayerContent::Raster(_))), "{:?}", r.document.layers); +} + /// 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.