diff --git a/src-tauri/src/commands/file_dialogs.rs b/src-tauri/src/commands/file_dialogs.rs index cff7e80..bc505eb 100644 --- a/src-tauri/src/commands/file_dialogs.rs +++ b/src-tauri/src/commands/file_dialogs.rs @@ -40,12 +40,47 @@ pub fn save_file_dialog(title: String, filename: String, filters: Vec) - } } +// security: write_binary_file / read_binary_file used to accept arbitrary +// paths with no validation, no dialog, no scope. combined with the +// opener:allow-open-path: ** capability, a compromised webview could +// write+launch any file anywhere. (LCEL-01) +// scope to the launcher's app data dir. legitimate callers (plugin +// loading) operate inside this dir anyway. +use tauri::Manager; + +fn is_path_scoped(path: &std::path::Path, app: &tauri::AppHandle) -> bool { + let canonical = match std::fs::canonicalize(path) { + Ok(p) => p, + Err(_) => return false, + }; + if let Ok(app_dir) = app.path().app_data_dir() { + let app_canonical = std::fs::canonicalize(&app_dir).unwrap_or(app_dir); + return canonical.starts_with(&app_canonical); + } + false +} + #[tauri::command] -pub fn write_binary_file(path: String, data: Vec) -> Result<(), String> { +pub fn write_binary_file( + app: tauri::AppHandle, + path: String, + data: Vec, +) -> Result<(), String> { + let p = std::path::Path::new(&path); + if !is_path_scoped(p, &app) { + return Err(format!("path outside app dir: {}", path)); + } fs::write(path, data).map_err(|e| e.to_string()) } #[tauri::command] -pub fn read_binary_file(path: String) -> Result, String> { +pub fn read_binary_file( + app: tauri::AppHandle, + path: String, +) -> Result, String> { + let p = std::path::Path::new(&path); + if !is_path_scoped(p, &app) { + return Err(format!("path outside app dir: {}", path)); + } fs::read(path).map_err(|e| e.to_string()) } diff --git a/src-tauri/src/commands/game.rs b/src-tauri/src/commands/game.rs index 0f49608..0093dd0 100644 --- a/src-tauri/src/commands/game.rs +++ b/src-tauri/src/commands/game.rs @@ -15,6 +15,50 @@ use crate::state::GameState; use crate::types::McServer; use crate::util; use crate::workshop_server; +// security: allowlist of game CLI flags the launcher will forward. +// extra_args from the frontend pass through to the spawned game and +// Wine/Proton honor many flags influencing DLL loading, debug ports, +// etc. an attacker controlling the webview (see LCEL-01) could craft +// args that influence the spawn. (LCEL-05) +const ALLOWED_EXTRA_ARG_PREFIXES: &[&str] = &[ + "--server", "--port", "--world", "--seed", "--gamemode", + "--difficulty", "--max-players", "--allow-cheats", + "--resource-pack", "--texture-pack", "--locale", +]; +const ALLOWED_EXTRA_ARG_EXACT: &[&str] = &[ + "--demo", "--bonus", "--debug", "--no-update", +]; + +fn filter_extra_args(args: &[String]) -> Vec { + let mut out = Vec::new(); + let mut i = 0; + while i < args.len() { + let a = &args[i]; + let exact_ok = ALLOWED_EXTRA_ARG_EXACT.iter().any(|p| *p == a); + let prefix_ok = ALLOWED_EXTRA_ARG_PREFIXES.iter().any(|p| a.starts_with(p)); + if exact_ok { + out.push(a.clone()); + i += 1; + } else if prefix_ok { + out.push(a.clone()); + // if the flag takes a value, also forward the next arg + if !a.contains('=') && i + 1 < args.len() { + let next = &args[i + 1]; + if !next.starts_with('--') { + out.push(next.clone()); + i += 2; + continue; + } + } + i += 1; + } else { + // skip unknown flag + i += 1; + } + } + out +} + #[tauri::command] #[allow(non_snake_case)] pub async fn launch_game( @@ -24,6 +68,7 @@ pub async fn launch_game( mut servers: Vec, extra_args: Vec, ) -> Result<(), String> { + let extra_args = filter_extra_args(&extra_args); perform_instance_sync(&app, &instance_id).await?; let working_dir = util::get_instance_working_dir(&app, &instance_id); let config_val = config::load_config_raw(app.clone()); diff --git a/src-tauri/src/commands/plugins.rs b/src-tauri/src/commands/plugins.rs index 9fdf26a..bb80be2 100644 --- a/src-tauri/src/commands/plugins.rs +++ b/src-tauri/src/commands/plugins.rs @@ -14,7 +14,30 @@ pub fn get_plugins_dir(app: tauri::AppHandle) -> Result { } #[tauri::command] -pub fn list_directory(path: String) -> Result, String> { +pub fn list_directory( + app: tauri::AppHandle, + path: String, +) -> Result, String> { + // security: list_directory used to accept arbitrary paths with no + // validation. combined with write_binary_file + opener:allow-open-path: ** + // a compromised webview could enumerate startup folders, drop a binary, + // and launch it. (LCEL-01) + // scope to the launcher's app data dir. legitimate callers (plugin + // discovery) operate inside this dir anyway. + use tauri::Manager; + let p = std::path::Path::new(&path); + let canonical = match std::fs::canonicalize(p) { + Ok(c) => c, + Err(e) => return Err(format!("path not found: {}", e)), + }; + if let Ok(app_dir) = app.path().app_data_dir() { + let app_canonical = std::fs::canonicalize(&app_dir).unwrap_or(app_dir); + if !canonical.starts_with(&app_canonical) { + return Err(format!("path outside app dir: {}", path)); + } + } else { + return Err("no app dir".into()); + } let entries = fs::read_dir(&path).map_err(|e| e.to_string())?; let mut results = Vec::new(); for entry in entries { diff --git a/src-tauri/src/commands/proxy_cmd.rs b/src-tauri/src/commands/proxy_cmd.rs index 5002c82..e5af82d 100644 --- a/src-tauri/src/commands/proxy_cmd.rs +++ b/src-tauri/src/commands/proxy_cmd.rs @@ -1,4 +1,34 @@ use crate::types::HttpResponse; +// security: allowlist of hosts http_proxy_request is allowed to talk to. +// the command used to accept any URL with no validation - full SSRF to +// loopback (127.0.0.1:5582 workshop server), LAN peers' relay ports, +// cloud metadata (169.254.169.254), internal admin panels. (LCEL-03) +const ALLOWED_HOSTS: &[&str] = &[ + "auth.mclegacyedition.xyz", + "social.mclegacyedition.xyz", + "raw.githubusercontent.com", + "api.github.com", +]; + +fn is_url_allowed(url: &str) -> bool { + let parsed = match reqwest::Url::parse(url) { + Ok(u) => u, + Err(_) => return false, + }; + let host = match parsed.host_str() { + Some(h) => h.to_lowercase(), + None => return false, + }; + // block loopback and link-local by default + if host == "localhost" || host == "127.0.0.1" || host == "::1" + || host.starts_with("127.") || host.starts_with("169.254.") + || host.starts_with("192.168.") || host.starts_with("10.") + { + return false; + } + ALLOWED_HOSTS.iter().any(|h| host == *h) +} + #[tauri::command] pub async fn http_proxy_request( method: String, @@ -6,6 +36,9 @@ pub async fn http_proxy_request( body: Option, headers: std::collections::HashMap, ) -> Result { + if !is_url_allowed(&url) { + return Err(format!("url not allowed: {}", url)); + } let client = reqwest::Client::new(); let mut req = match method.to_uppercase().as_str() { "GET" => client.get(&url), diff --git a/src-tauri/src/commands/workshop.rs b/src-tauri/src/commands/workshop.rs index 876f9a4..2cf2810 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,38 @@ 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: assert the resolved path stays inside instance_dir. + // canonicalize() requires the path to exist, so we canonicalize + // the existing parent and append the basename. (LCEL-02) + let instance_canon = instance_dir.canonicalize() + .unwrap_or_else(|_| instance_dir.clone()); + let check_path = if let Some(parent) = resolved.parent() { + let parent_canon = parent.canonicalize() + .unwrap_or_else(|_| parent.to_path_buf()); + let basename = resolved.file_name() + .ok_or_else(|| format!("invalid placeholder: {}", placeholder))?; + parent_canon.join(basename) + } else { + resolved.clone() + }; + if !check_path.starts_with(&instance_canon) { + let _ = fs::remove_dir_all(&tmp_dir); + return Err(format!("placeholder escapes instance dir: {}", placeholder)); + } PathBuf::from(resolved) }; @@ -178,6 +214,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); }