fix(svg): refuse a document compressed twice and charge a refused embedded one (#1924)

One gzip layer is unpacked within the import budget, but usvg gunzips what
it is handed again with no limit, so a doubly compressed document or
embedded image expanded outside the budget. A compressed embedded image
that failed its budget also left the shared allowance untouched, so every
sibling image could repeat the work; a refusal now spends what was left.
This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-03 22:26:21 -05:00
1 parent 9475a815da
commit 6da4c30cae
2 files changed
+42 -1

No files matched your search

+14 -1
View File
@@ -44,6 +44,11 @@ fn inflate(data: &[u8], limit: usize) -> Result<Vec<u8>, String> {
if out.len() > limit {
return Err(format!("compressed SVG expands past {} MiB", limit >> 20));
}
// usvg gunzips what it is handed, with no limit: one layer is unpacked here, within the
// budget, and what comes out must be the document itself.
if gzipped(&out) {
return Err("compressed SVG holds another compressed file, not a document".into());
}
Ok(out)
}
@@ -72,7 +77,15 @@ fn embedded(used: &Used, limits: &Limits, data: &[u8], opts: &Options) -> Option
return None;
}
let left = limits.embedded.saturating_sub(used.bytes.load(Ordering::Relaxed));
let plain = inflate(data, left).ok()?;
let plain = match inflate(data, left) {
Ok(plain) => plain,
// What a refused document unpacked before it was refused was work too: it uses up what is
// left, so the next image can't repeat it.
Err(_) => {
used.bytes.fetch_add(left, Ordering::Relaxed);
return None;
}
};
used.bytes.fetch_add(plain.len(), Ordering::Relaxed);
if super::too_deep(&plain) {
return None;
+28
View File
@@ -111,3 +111,31 @@ fn linked_files_and_other_mime_types_stay_unresolved() {
);
assert_eq!(layers(data.as_bytes(), Limits::default()).unwrap(), layers(svg("").as_bytes(), Limits::default()).unwrap());
}
#[test]
fn a_document_compressed_twice_is_refused_not_unpacked_again() {
// One gzip layer is unpacked within the budget; usvg would unpack a second one without any.
let twice = gzip(&gzip(svg(&" ".repeat(1 << 20)).as_bytes()));
let e = layers(&twice, Limits { document: 4 << 20, ..Limits::default() }).unwrap_err();
assert!(e.contains("another compressed file"), "{e}");
// Embedded, the same is dropped.
let inner = svg(r##"<rect width="10" height="10"/>"##);
let data = svg(&format!(
r##"<image width="10" height="10" xlink:href="data:image/svg+xml;base64,{}"/>"##,
base64::engine::general_purpose::STANDARD.encode(gzip(&gzip(inner.as_bytes())))
))
.into_bytes();
assert_eq!(layers(&data, Limits::default()).unwrap(), layers(svg("").as_bytes(), Limits::default()).unwrap());
}
#[test]
fn a_refused_embedded_bomb_uses_up_the_budget_for_the_images_after_it() {
let bomb = gzip(svg(&" ".repeat(1 << 20)).as_bytes());
let inner = svg(r##"<rect width="10" height="10"/>"##);
let image = |href: String| format!(r##"<image width="10" height="10" xlink:href="{href}"/>"##);
let bomb_uri = format!("data:image/svg+xml;base64,{}", base64::engine::general_purpose::STANDARD.encode(&bomb));
let data = svg(&format!("{}{}", image(bomb_uri).repeat(8), image(uri(inner.as_bytes())))).into_bytes();
let limits = Limits { embedded: 4096, ..Limits::default() };
let empty = layers(svg("").as_bytes(), limits).unwrap();
assert_eq!(layers(&data, limits).unwrap(), empty, "the first bomb spends the allowance; nothing after it unpacks");
}