From a5186f5a12c1d578b7b7aae0a0a909393f0123f4 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:16:12 +0200 Subject: [PATCH 1/2] Keep Export for Screens suffix inside the export folder (#1713) --- crates/engine/src/cmd/fileio/screens.rs | 10 ++++++++-- crates/engine/src/cmd/fileio/tests_screens.rs | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/fileio/screens.rs b/crates/engine/src/cmd/fileio/screens.rs index b1191520..1c5a8a7c 100644 --- a/crates/engine/src/cmd/fileio/screens.rs +++ b/crates/engine/src/cmd/fileio/screens.rs @@ -141,6 +141,12 @@ fn safe_name(s: &str) -> Option { (!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 { } 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)?; diff --git a/crates/engine/src/cmd/fileio/tests_screens.rs b/crates/engine/src/cmd/fileio/tests_screens.rs index 0ff69b39..272724ba 100644 --- a/crates/engine/src/cmd/fileio/tests_screens.rs +++ b/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"))] From d82d6e36204867e65cca75ba54740bdf4d513ffa Mon Sep 17 00:00:00 2001 From: bflatastic Date: Wed, 14 Oct 2026 02:09:14 +0700 Subject: [PATCH 2/2] Test that a hostile Export for Screens suffix or prefix stays in the export folder (#1713) --- crates/engine/src/cmd/fileio/tests_screens.rs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/crates/engine/src/cmd/fileio/tests_screens.rs b/crates/engine/src/cmd/fileio/tests_screens.rs index 272724ba..7dc2e7de 100644 --- a/crates/engine/src/cmd/fileio/tests_screens.rs +++ b/crates/engine/src/cmd/fileio/tests_screens.rs @@ -123,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() {