Merge pull request #1777 from MarekSpodBiedry/issue-1713

Keep Export for Screens suffix inside the export folder (#1713)

crates/engine/src/cmd/fileio/screens.rs

@@ -141,6 +141,12 @@ fn safe_name(s: &str) -> Option<String> {
     (!n.trim_matches('.').is_empty()).then_some(n)
 }
 
+/// Text that goes into a file name (prefix, suffix): path separators and `:` become `-`, so it can't
+/// start a folder or climb out of the export folder.
+fn file_part(s: &str) -> String {
+    s.chars().map(|c| if matches!(c, '/' | '\\' | ':') { '-' } else { c }).collect()
+}
+
 /// Keys of a format row that aren't encoder options.
 const ROW_KEYS: [&str; 7] = ["format", "scale", "ppi", "width", "height", "suffix", "folder"];
 
@@ -169,7 +175,7 @@ impl Row {
         let size = if format.raster { ScreenSize::of_row(row)? } else { ScreenSize::Scale(1.0) };
         let suffix = match str_param(row, "suffix") {
             Some(s) if !format.raster && is_size_suffix(s) => String::new(),
-            Some(s) => s.to_string(),
+            Some(s) => file_part(s),
             None => size.suffix(),
         };
         let folder = match str_param(row, "folder").and_then(safe_name) {
@@ -336,7 +342,7 @@ pub(crate) fn export(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let subfolders = o.subfolders || o.preset.is_some();
     // A prefix names files, not folders.
-    let prefix: String = o.prefix.as_deref().unwrap_or("").chars().map(|c| if matches!(c, '/' | '\\' | ':') { '-' } else { c }).collect();
+    let prefix = file_part(o.prefix.as_deref().unwrap_or(""));
     let folder = o.folder.as_deref().filter(|f| !f.is_empty());
     if let Some(dir) = folder {
         create_dir(dir)?;

crates/engine/src/cmd/fileio/tests_screens.rs

@@ -105,6 +105,9 @@ fn density_buckets_go_into_sub_folders() {
     // A prefix names files, not folders.
     let r = export(&mut s, json!({"range": "2", "prefix": "../a\\b:", "formats": [{"format": "svg"}]}));
     assert_eq!(names(&r), ["..-a-b-Artboard-2.svg"]);
+    // So does a suffix: it can't open a folder or climb out of the export folder.
+    let r = export(&mut s, json!({"range": "2", "formats": [{"format": "svg", "suffix": "/../x\\..\\y"}, {"format": "png", "suffix": "-ok"}]}));
+    assert_eq!(names(&r), ["Artboard-2-..-x-..-y.svg", "Artboard-2-ok.png"]);
 }
 
 #[cfg(not(target_arch = "wasm32"))]
@@ -120,6 +123,35 @@ fn sub_folders_are_made_in_the_export_folder() {
     let _ = std::fs::remove_dir_all(dir);
 }
 
+/// #1713: whatever a suffix or prefix says (`..`, an absolute path, a drive, a share), every file
+/// lands directly in the export folder.
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn a_hostile_suffix_or_prefix_stays_in_the_export_folder() {
+    let mut s = session(1);
+    let top = std::env::temp_dir().join(format!("vc-screens-escape-{}", std::process::id()));
+    let dir = top.join("out");
+    let folder = dir.to_string_lossy().replace('\\', "/");
+    for text in ["/../../up", "\\..\\up", "/etc/x", "C:\\Windows\\x", "\\\\server\\share\\x", "/..", ".."] {
+        for key in ["suffix", "prefix"] {
+            let mut p = json!({"folder": folder, "formats": [{"format": "svg"}]});
+            if key == "suffix" {
+                p["formats"][0][key] = json!(text);
+            } else {
+                p[key] = json!(text);
+            }
+            let r = export(&mut s, p);
+            let written = std::path::PathBuf::from(r["files"][0].as_str().unwrap());
+            assert!(written.is_file(), "{key} {text}");
+            assert_eq!(written.parent().unwrap().canonicalize().unwrap(), dir.canonicalize().unwrap(), "{key} {text}");
+        }
+    }
+    // Nothing beside the export folder, no folder inside it.
+    assert_eq!(std::fs::read_dir(&top).unwrap().count(), 1);
+    assert!(std::fs::read_dir(&dir).unwrap().all(|e| e.unwrap().path().is_file()));
+    let _ = std::fs::remove_dir_all(top);
+}
+
 #[cfg(not(target_arch = "wasm32"))]
 #[test]
 fn a_zip_is_written_into_the_folder() {