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.
This commit is contained in:
M1noa 2026-07-16 17:42:48 -05:00
parent 8da6a217bc
commit 69998a6ae4

View file

@ -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);
}