Merge pull request #8505 from dylanmtaylor/security/svg-svgz-double-decompress

security: stop double decompression of gzip-compressed SVG

crates/io/src/svg.rs

@@ -194,6 +194,18 @@ fn options(fontdb: Arc<usvg::fontdb::Database>) -> 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<usvg::fontdb::Database>) -> 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<u8> {
+            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"<svg xmlns=\"http://www.w3.org/2000/svg\" width=\"4\" height=\"4\"><rect width=\"4\" height=\"4\"/></svg>";
+        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());
+    }
 }

crates/io/tests/it/svg.rs

@@ -282,6 +282,52 @@ fn gzip(bytes: &[u8]) -> Vec<u8> {
     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!("<svg {NS} width=\"4\" height=\"4\"><rect width=\"4\" height=\"4\" fill=\"#00ff00\"/></svg>");
+    let url = base64_data_url("image/svg+xml", &gzip(inner.as_bytes()));
+    let r = open(&format!("<svg {NS} width=\"10\" height=\"10\"><image x=\"0\" y=\"0\" width=\"10\" height=\"10\" href=\"{url}\"/></svg>"));
+    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!("<svg {NS} width=\"4\" height=\"4\">").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 url = base64_data_url("image/svg+xml", &gz);
+    let start = std::time::Instant::now();
+    let r = open(&format!("<svg {NS} width=\"10\" height=\"10\"><image x=\"0\" y=\"0\" width=\"10\" height=\"10\" href=\"{url}\"/></svg>"));
+    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.