Merge pull request #5375 from storytold/fix/sec-recovery-read-plain-files

Data Recovery: read only plain files in this account's own areas

crates/engine/src/cmd/recovery.rs

@@ -308,6 +308,14 @@ impl RecoveryStore for FolderStore {
     }
     fn read(&self, name: &str) -> std::result::Result<Vec<u8>, String> {
         let f = self.file(name)?;
+        // Entry names come from the copy's metadata: refuse a link (or a folder another account
+        // owns) that would read a file from outside the store.
+        if let Some(dir) = f.parent() {
+            Self::own_area(dir)?;
+        }
+        if !std::fs::symlink_metadata(&f).map_err(|e| format!("{}: {e}", f.display()))?.file_type().is_file() {
+            return Err(format!("{}: not a plain file", f.display()));
+        }
         std::fs::read(&f).map_err(|e| format!("{}: {e}", f.display()))
     }
     fn write(&self, name: &str, bytes: &[u8]) -> std::result::Result<(), String> {

crates/engine/src/tests/recovery.rs

@@ -417,6 +417,21 @@ fn the_folder_store_makes_owner_only_folders() {
     let _ = std::fs::remove_dir_all(&base);
 }
 
+/// An entry that is a link (the copy's metadata names it) is not followed out of the store.
+#[cfg(unix)]
+#[test]
+fn the_folder_store_reads_no_linked_entries() {
+    let base = std::env::temp_dir().join(format!("vc-recovery-links-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&base);
+    let store = FolderStore::new(&base);
+    store.write("1-1/Untitled-1-1.vectorcraft", b"copy").unwrap();
+    std::fs::write(base.join("outside"), b"private").unwrap();
+    std::os::unix::fs::symlink(base.join("outside"), base.join("1-1").join("img")).unwrap();
+    assert_eq!(store.read("1-1/Untitled-1-1.vectorcraft").unwrap(), b"copy");
+    assert!(store.read("1-1/img").is_err());
+    let _ = std::fs::remove_dir_all(&base);
+}
+
 #[test]
 fn saving_reverting_or_closing_removes_the_copy() {
     let dir = std::env::temp_dir().join(format!("vc-recovery-save-{}", std::process::id()));