diff --git a/crates/format/src/image_tests.rs b/crates/format/src/image_tests.rs index bc925efbf..e58a4ab16 100644 --- a/crates/format/src/image_tests.rs +++ b/crates/format/src/image_tests.rs @@ -86,3 +86,29 @@ fn many_lost_images_are_counted_not_listed_one_by_one() { assert_eq!(issues.len(), MAX_IMAGE_ISSUES + 1, "{issues:?}"); assert_eq!(issues.last().map(String::as_str), Some("20 more images couldn't be loaded")); } + +#[test] +fn image_members_with_path_like_names_are_dropped_and_not_saved_back() { + let (d, hash) = doc_with_image(); + let bytes = save_bytes(&d, None).unwrap(); + let mut z = zip::ZipArchive::new(std::io::Cursor::new(&bytes)).unwrap(); + let doc_json = read_member(&mut z, "document.json", u64::MAX).unwrap(); + let mut buf = std::io::Cursor::new(Vec::new()); + let mut w = zip::ZipWriter::new(&mut buf); + let opts = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + w.start_file("document.json", opts).unwrap(); + w.write_all(&doc_json).unwrap(); + for n in ["images/../../x", "images/a/b"] { + w.start_file(n, opts).unwrap(); + w.write_all(&png()).unwrap(); + } + w.finish().unwrap(); + let hostile = buf.into_inner(); + let loaded = load_bytes_with_versions(&hostile).unwrap(); + assert!(loaded.doc.images.keys().all(|k| k != "../../x" && k != "a/b"), "{:?}", loaded.doc.images.keys()); + assert!(!loaded.issues.is_empty()); + let saved = save_bytes_with_history(&loaded.doc, None, &loaded.versions, &loaded.branches).unwrap(); + let z = zip::ZipArchive::new(std::io::Cursor::new(saved)).unwrap(); + assert!(z.file_names().all(|n| !n.contains("..") && n.matches('/').count() <= 1), "{:?}", z.file_names().collect::>()); + let _ = hash; +} diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 9f7e51bc4..36975f56b 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -524,10 +524,18 @@ fn read_images( own: Option<&BTreeSet>, budget: ImageBudget, ) -> (BTreeMap, Vec<(String, FormatError)>) { - let mut hashes: Vec = z.file_names().filter_map(|n| n.strip_prefix("images/")).map(str::to_string).collect(); + let mut hashes: Vec = z.file_names().filter_map(|n| n.strip_prefix("images/")).filter(|h| !h.is_empty()).map(str::to_string).collect(); hashes.sort_by_key(|h| !own.is_some_and(|o| o.contains(h))); let mut images = BTreeMap::new(); let mut unread = Vec::new(); + // Member names that aren't plain hashes (`../x`, `a/b`) are never read, so never written back. + hashes.retain(|h| { + let ok = library::is_valid_hash(h); + if !ok { + unread.push((h.clone(), FormatError::Corrupt("its name isn't a valid image hash".to_string()))); + } + ok + }); let mut left = budget.bytes; for (i, hash) in hashes.into_iter().enumerate() { let name = format!("images/{hash}"); diff --git a/crates/format/src/library.rs b/crates/format/src/library.rs index 06dcc99e6..daec05e2c 100644 --- a/crates/format/src/library.rs +++ b/crates/format/src/library.rs @@ -39,7 +39,7 @@ pub fn is_valid_library_id(id: &str) -> bool { } /// Image hashes become archive member names: only plain hex-like names are looked up. -fn is_valid_hash(h: &str) -> bool { +pub(crate) fn is_valid_hash(h: &str) -> bool { (1..=128).contains(&h.len()) && h.bytes().all(|b| b.is_ascii_alphanumeric()) }