diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 15156f8ad..3629eb893 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -177,20 +177,36 @@ pub fn load(bytes: &[u8]) -> Result { /// Most entries a `.designcraft` archive may hold. const MAX_ENTRIES: usize = 100_000; -/// Most bytes `meta.json` or `document.json` may expand to. -const MAX_JSON_BYTES: u64 = 256 << 20; -/// Most bytes one embedded asset may expand to. -const MAX_ASSET_BYTES: u64 = 1 << 30; -/// Most bytes all the entries read from one archive may expand to together. -const MAX_TOTAL_BYTES: u64 = 4 << 30; -/// Read entry `name` of `z`, or `None` when it is missing or unreadable. Refuses an entry that -/// expands past `limit` bytes or past what is `left` of the archive's budget, without -/// decompressing more than that: the sizes the archive claims are not trusted. +/// How much a `.designcraft` archive may expand to when read. The sizes an archive claims are +/// not trusted, so deflated entries are capped; a *stored* entry can't expand (its size is its +/// bytes in the archive, which is already in memory, and all stored entries together can't +/// exceed the archive), so a placed image — stored as is by [`save`] — opens whatever its size +/// and counts for nothing against the budget. Saving has no cap, and this keeps every saved +/// document openable (#2166). +#[derive(Clone, Copy, Debug)] +struct Limits { + /// Most bytes `meta.json` or `document.json` may expand to. + json: u64, + /// Most bytes one deflated asset may expand to. + asset: u64, + /// Most bytes all the deflated entries read from one archive may expand to together. + total: u64, +} + +impl Limits { + const DEFAULT: Limits = Limits { json: 256 << 20, asset: 1 << 30, total: 4 << 30 }; +} + +/// Read entry `name` of `z`, or `None` when it is missing or unreadable. Refuses a deflated entry +/// that expands past `limit` bytes or past what is `left` of the archive's budget, without +/// decompressing more than that: the sizes the archive claims are not trusted. A stored entry +/// is read at its size in the archive and leaves the budget alone. fn read_entry(z: &mut zip::ZipArchive>, name: &str, limit: u64, left: &mut u64) -> Result>, FormatError> { - let too_big = || FormatError::Io(format!("{name} is larger than the {limit}-byte limit (or the archive is larger than {MAX_TOTAL_BYTES} bytes)")); let Ok(f) = z.by_name(name) else { return Ok(None) }; - let limit = limit.min(*left); + let stored = f.compression() == zip::CompressionMethod::Stored; + let limit = if stored { f.compressed_size() } else { limit.min(*left) }; + let too_big = || FormatError::Io(format!("{name} is larger than the {limit}-byte limit (or the archive expands past its {}-byte budget)", *left)); if f.size() > limit { return Err(too_big()); } @@ -202,24 +218,30 @@ fn read_entry(z: &mut zip::ZipArchive>, name: &str, limit: u64, le if n > limit { return Err(too_big()); } - *left = left.saturating_sub(n); + if !stored { + *left = left.saturating_sub(n); + } Ok(Some(v)) } fn load_zip(bytes: &[u8]) -> Result { + load_zip_within(bytes, Limits::DEFAULT) +} + +fn load_zip_within(bytes: &[u8], limits: Limits) -> Result { let mut z = zip::ZipArchive::new(Cursor::new(bytes)).map_err(|e| FormatError::NotOurs(e.to_string()))?; if z.len() > MAX_ENTRIES { return Err(FormatError::NotOurs(format!("{} entries (at most {MAX_ENTRIES})", z.len()))); } - let mut left = MAX_TOTAL_BYTES; - let version = read_entry(&mut z, "meta.json", MAX_JSON_BYTES, &mut left)? + let mut left = limits.total; + let version = read_entry(&mut z, "meta.json", limits.json, &mut left)? .and_then(|meta| serde_json::from_slice::(&meta).ok()) .and_then(|m| m.get("version").and_then(Value::as_u64)) .unwrap_or(1); if version > u64::from(VERSION) { return Err(FormatError::TooNew(version)); } - let json = read_entry(&mut z, "document.json", MAX_JSON_BYTES, &mut left)?.ok_or_else(|| FormatError::NotOurs("missing document.json".into()))?; + let json = read_entry(&mut z, "document.json", limits.json, &mut left)?.ok_or_else(|| FormatError::NotOurs("missing document.json".into()))?; let mut v: Value = serde_json::from_slice(&json).map_err(|e| FormatError::NotOurs(e.to_string()))?; upgrade_json(&mut v); let mut doc: Document = serde_json::from_value(v).map_err(|e| FormatError::NotOurs(e.to_string()))?; @@ -235,7 +257,7 @@ fn load_zip(bytes: &[u8]) -> Result { if !doc.assets.contains_key(&AssetId(id)) { continue; } - if let (Some(data), Some(a)) = (read_entry(&mut z, &name, MAX_ASSET_BYTES, &mut left)?, doc.assets.get_mut(&AssetId(id))) { + if let (Some(data), Some(a)) = (read_entry(&mut z, &name, limits.asset, &mut left)?, doc.assets.get_mut(&AssetId(id))) { let same = loaded.entry(fingerprint.hash_one(data.as_slice())).or_default(); let data = match same.iter().find(|d| ***d == data) { Some(d) => d.clone(), diff --git a/crates/format/src/tests.rs b/crates/format/src/tests.rs index d4b838cf2..a95bee160 100644 --- a/crates/format/src/tests.rs +++ b/crates/format/src/tests.rs @@ -54,6 +54,85 @@ fn save_uncompacted(d: &Document) -> Vec { buf.into_inner() } +/// A `.designcraft` with the given asset entries, each stored as is or deflated. +fn archive_with(d: &Document, entries: &[(AssetId, &[u8], bool)]) -> Vec { + let mut buf = Cursor::new(Vec::new()); + let mut z = zip::ZipWriter::new(&mut buf); + let stored = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + let deflate = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); + z.start_file("mimetype", stored).unwrap(); + z.write_all(MIME.as_bytes()).unwrap(); + z.start_file("meta.json", deflate).unwrap(); + z.write_all(json!({"format": "designcraft", "version": VERSION}).to_string().as_bytes()).unwrap(); + z.start_file("document.json", deflate).unwrap(); + z.write_all(&serde_json::to_vec(d).unwrap()).unwrap(); + for (id, data, as_is) in entries { + z.start_file(format!("assets/{}.png", id.0), if *as_is { stored } else { deflate }).unwrap(); + z.write_all(data).unwrap(); + } + z.finish().unwrap(); + buf.into_inner() +} + +/// Bytes that don't compress (a simple generator, so the test needs no crate). +fn noise(n: usize) -> Vec { + let mut x: u32 = 0x9e37_79b9; + (0..n) + .map(|_| { + x ^= x << 13; + x ^= x >> 17; + x ^= x << 5; + (x >> 24) as u8 + }) + .collect() +} + +/// #2166: saving has no size cap, so opening must keep every saved document openable. A stored +/// entry (how placed images are saved) can't expand and opens at any size; the total budget is +/// never below the archive's own size. Deflated entries stay capped, so a zip bomb is refused. +#[test] +fn stored_assets_open_at_any_size_while_deflated_ones_stay_capped() { + let mut d = Document::new(&NewDocument::default()); + let big = noise(50_000); + let a = png_asset(&mut d, &big); + let b = png_asset(&mut d, &big); + d.assets.values_mut().for_each(|a| Arc::make_mut(a).data = Arc::new(Vec::new())); + place(&mut d, a, 0.0); + place(&mut d, b, 0.0); + // A budget that fits the document's JSON but not the two 50 000-byte assets together. + let json_len = serde_json::to_vec(&d).unwrap().len() as u64; + let tight = Limits { json: 256 << 20, asset: 1_000, total: json_len + 10_000 }; + assert!(tight.total < 100_000, "the JSON is {json_len} bytes; the budget must stay under the two assets"); + + // Two stored assets, each over the asset cap and together over the total: both open. + let bytes = archive_with(&d, &[(a, &big, true), (b, &big, true)]); + let doc = load_zip_within(&bytes, tight).expect("stored entries open at their own size"); + assert_eq!(doc.assets[&a].data.len(), 50_000); + // (Identical bytes come back as one asset: `load` merges duplicates.) + assert_eq!(doc.assets.len(), 1); + assert!(doc.all_items().iter().filter_map(|i| doc.item(*i)?.graphic()).all(|g| g.asset == a), "both placements show it"); + + // A deflated entry past the cap is refused, whether it really is that big ... + let bytes = archive_with(&d, &[(a, &big, false), (b, &big, true)]); + let e = load_zip_within(&bytes, tight).expect_err("a deflated entry over the cap").to_string(); + assert!(e.contains("larger than the 1000-byte limit"), "{e}"); + // ... or claims to be (a few KB that expand to megabytes). + let bomb = vec![0u8; 3 << 20]; + let bytes = archive_with(&d, &[(a, &bomb, false), (b, &big, true)]); + assert!(bytes.len() < 64 << 10, "the bomb is small on disk: {}", bytes.len()); + assert!(load_zip_within(&bytes, tight).is_err(), "a zip bomb is refused"); + + // The default limits read the same stored archive, and what `save` writes round-trips. + let bytes = archive_with(&d, &[(a, &big, true), (b, &big, true)]); + assert_eq!(load_zip(&bytes).unwrap().assets[&a].data.len(), 50_000); + Arc::make_mut(d.assets.get_mut(&a).unwrap()).data = Arc::new(big.clone()); + Arc::make_mut(d.assets.get_mut(&b).unwrap()).data = Arc::new(big.clone()); + let saved = save(&d).unwrap(); + let doc = load_zip_within(&saved, tight).expect("what save writes opens under a cap smaller than its images"); + // (`save` merges the two identical assets into one.) + assert!(!doc.assets.is_empty() && doc.assets.values().all(|x| x.data.len() == 50_000), "every saved image is back"); +} + #[test] fn explicit_all_on_pdf_layers_require_a_capable_reader_only_when_needed() { let mut d = Document::new(&NewDocument::default());