Validate image member names when reading .figcraft archives (#1148)

Fixes #1122

Co-authored-by: Claude Haiku 5.5 <noreply@anthropic.com>

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::<Vec<_>>());
+    let _ = hash;
+}

crates/format/src/lib.rs

@@ -524,10 +524,18 @@ fn read_images<R: Read + Seek>(
     own: Option<&BTreeSet<String>>,
     budget: ImageBudget,
 ) -> (BTreeMap<String, ImageData>, Vec<(String, FormatError)>) {
-    let mut hashes: Vec<String> = z.file_names().filter_map(|n| n.strip_prefix("images/")).map(str::to_string).collect();
+    let mut hashes: Vec<String> = 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}");

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())
 }