From 69998a6ae46e1771cb78f7a2c3611eb3a5af54a4 Mon Sep 17 00:00:00 2001 From: M1noa Date: Thu, 16 Jul 2026 17:42:48 -0500 Subject: [PATCH] fix(LCEL-02): validate workshop install paths + uninstall paths three changes to workshop_install / workshop_uninstall: 1. zip_name from the frontend is now rejected if it contains '..', '/', '\\', or starts with '/' or '\\'. prevents the downloaded archive from escaping tmp_dir. 2. placeholder is now rejected if it contains '..' or starts with '/' or '\\'. after joining to instance_dir, the resolved path is canonicalized and asserted to start with instance_dir.canonicalize(). prevents the install from escaping the instance dir. 3. workshop_uninstall now re-validates every persisted path in pkg.dirs against instance_dir.canonicalize() before removing. if an attacker injected paths into workshop_packages.json (via the install traversals above or via LCEL-01's write primitive), uninstall used to delete arbitrary files. the proper fix is to use a safe zip library (zip-rs) that strips '..' from members instead of shelling to bsdtar/unzip/tar. left as a TODO so this commit stays minimal and reviewable. --- src-tauri/src/commands/workshop.rs | 41 ++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src-tauri/src/commands/workshop.rs b/src-tauri/src/commands/workshop.rs index 876f9a4..d45ca23 100644 --- a/src-tauri/src/commands/workshop.rs +++ b/src-tauri/src/commands/workshop.rs @@ -35,6 +35,15 @@ pub async fn workshop_install(app: AppHandle, request: WorkshopInstallRequest) - for (zip_name, placeholder) in &request.zips { let zip_url = format!("{}/{}", raw_base, zip_name); let zip_tmp = tmp_dir.join(zip_name); + // security: zip_name is user/registry-controlled. reject any path + // separator or .. segment so the downloaded archive can't escape + // tmp_dir. (LCEL-02) + if zip_name.contains("..") || zip_name.contains('/') || zip_name.contains('\\') + || zip_name.starts_with('/') || zip_name.starts_with('\\') + { + let _ = fs::remove_dir_all(&tmp_dir); + return Err(format!("invalid zip_name: {}", zip_name)); + } let response = reqwest::get(&zip_url).await.map_err(|e| e.to_string())?; if !response.status().is_success() { let _ = fs::remove_dir_all(&tmp_dir); @@ -45,11 +54,28 @@ pub async fn workshop_install(app: AppHandle, request: WorkshopInstallRequest) - let dest_dir = if placeholder.is_empty() { instance_dir.clone() } else { + // security: placeholder is user/registry-controlled. reject any + // .. segment or absolute path before joining to instance_dir. + // (LCEL-02) + if placeholder.contains("..") || placeholder.starts_with('/') + || placeholder.starts_with('\\') + { + let _ = fs::remove_dir_all(&tmp_dir); + return Err(format!("invalid placeholder: {}", placeholder)); + } let resolved = instance_dir.clone().join(placeholder .replace("{MediaDir}", media_dir.to_str().unwrap_or("")) .replace("{DLCDir}", dlc_dir.to_str().unwrap_or("")) .replace("{GameHDD}", game_hdd.to_str().unwrap_or("")) .replace("{MobDir}", mob_dir.to_str().unwrap_or(""))); + // security: canonicalize and assert the resolved path stays + // inside instance_dir. (LCEL-02) + let resolved_canon = resolved.canonicalize().unwrap_or(resolved.clone()); + let instance_canon = instance_dir.canonicalize().unwrap_or(instance_dir.clone()); + if !resolved_canon.starts_with(&instance_canon) { + let _ = fs::remove_dir_all(&tmp_dir); + return Err(format!("placeholder escapes instance dir: {}", placeholder)); + } PathBuf::from(resolved) }; @@ -178,6 +204,21 @@ pub async fn workshop_uninstall(app: AppHandle, instance_id: String, package_id: if let Some(pkg) = packages.iter().find(|p| p.id == package_id) { for file in &pkg.dirs { let path = PathBuf::from(file); + // security: re-validate that the persisted path stays inside + // instance_dir before removing. if an attacker injected paths + // into workshop_packages.json (via the install traversals above + // or via LCEL-01's write primitive), uninstall used to delete + // arbitrary files. (LCEL-02) + let path_canon = match path.canonicalize() { + Ok(p) => p, + Err(_) => continue, + }; + let instance_canon = instance_dir + .canonicalize() + .unwrap_or_else(|_| instance_dir.clone()); + if !path_canon.starts_with(&instance_canon) { + continue; + } if path.is_file() { let _ = fs::remove_file(&path); }