From d5a7c81fee87583eae7a84da8ed7ecb1b79e8baf Mon Sep 17 00:00:00 2001 From: M1noa Date: Thu, 16 Jul 2026 20:27:24 -0500 Subject: [PATCH] fix(LCEL-03): allowlist http_proxy_request hosts --- src-tauri/src/commands/file_dialogs.rs | 16 ++++++-- src-tauri/src/commands/game.rs | 7 ++++ src-tauri/src/commands/plugins.rs | 17 ++++---- src-tauri/src/commands/proxy_cmd.rs | 33 ++++++++++++++++ src-tauri/src/commands/workshop.rs | 41 +++++++++++++++++++ src-tauri/src/lib.rs | 54 +++++++++++++++----------- src-tauri/src/networking/relay.rs | 13 ++++++- src-tauri/src/workshop_server.rs | 5 ++- src/services/LceOnlineService.ts | 6 +++ 9 files changed, 154 insertions(+), 38 deletions(-) diff --git a/src-tauri/src/commands/file_dialogs.rs b/src-tauri/src/commands/file_dialogs.rs index cff7e80..16003c6 100644 --- a/src-tauri/src/commands/file_dialogs.rs +++ b/src-tauri/src/commands/file_dialogs.rs @@ -40,12 +40,20 @@ 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) +// TODO: scope these to a launcher-specific subdirectory with canonical +// containment checks, or remove from the IPC surface entirely and route +// all binary file ops through rfd-gated dialogs. +// for now, refuse all paths. callers must use pick_file / save_file_dialog. #[tauri::command] -pub fn write_binary_file(path: String, data: Vec) -> Result<(), String> { - fs::write(path, data).map_err(|e| e.to_string()) +pub fn write_binary_file(_path: String, _data: Vec) -> Result<(), String> { + Err("write_binary_file disabled: use save_file_dialog instead".into()) } #[tauri::command] -pub fn read_binary_file(path: String) -> Result, String> { - fs::read(path).map_err(|e| e.to_string()) +pub fn read_binary_file(_path: String) -> Result, String> { + Err("read_binary_file disabled: use pick_file instead".into()) } diff --git a/src-tauri/src/commands/game.rs b/src-tauri/src/commands/game.rs index 15627da..0f49608 100644 --- a/src-tauri/src/commands/game.rs +++ b/src-tauri/src/commands/game.rs @@ -87,6 +87,13 @@ pub async fn launch_game( } all_args.extend(args.clone()); all_args.push(game_exe.to_string_lossy().to_string()); + // security: extra_args from the frontend pass through to the + // spawned game verbatim. 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) + // TODO: allowlist extra_args against a known safe set + // (e.g. --server, --port). reject unknown flags. all_args.extend(extra_args.clone()); let (final_prog, final_args) = apply_launch_prefix(prog, all_args, &config_val); let mut cmd = tokio::process::Command::new(&final_prog); diff --git a/src-tauri/src/commands/plugins.rs b/src-tauri/src/commands/plugins.rs index 9fdf26a..91fa33c 100644 --- a/src-tauri/src/commands/plugins.rs +++ b/src-tauri/src/commands/plugins.rs @@ -15,16 +15,13 @@ pub fn get_plugins_dir(app: tauri::AppHandle) -> Result { #[tauri::command] pub fn list_directory(path: String) -> Result, String> { - let entries = fs::read_dir(&path).map_err(|e| e.to_string())?; - let mut results = Vec::new(); - for entry in entries { - let entry = entry.map_err(|e| e.to_string())?; - results.push(DirEntry { - name: entry.file_name().to_string_lossy().to_string(), - is_dir: entry.file_type().map(|t| t.is_dir()).unwrap_or(false), - }); - } - Ok(results) + // 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) + // TODO: scope to the launcher's plugins dir or remove from IPC entirely. + // for now, refuse all paths. + Err("list_directory disabled: use get_plugins_dir instead".into()) } #[tauri::command] 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..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); } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index e2852c6..e299e3c 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -129,29 +129,39 @@ pub fn run() { } let args: Vec = std::env::args().collect(); - if args.len() > 1 && !args[1].starts_with('-') { - let first = &args[1]; - let is_deep_link = first.starts_with("emerald://") - || first.starts_with("emeraldlauncher://") - || first.starts_with("discord-1482504445152460871://"); - if !is_deep_link { - let instance_id = first.clone(); - let app_handle_clone = app.handle().clone(); - tauri::async_runtime::spawn(async move { - if let Some(window) = app_handle_clone.get_webview_window("main") { - let _ = window.hide(); - } - let state = app_handle_clone.state::(); - match game::launch_game(app_handle_clone.clone(), state, instance_id, Vec::new(), vec![]).await { - Ok(_) => app_handle_clone.exit(0), - Err(e) => { - let _ = app_handle_clone.emit("backend-error", format!("Auto-launch: {e}")); - eprintln!("Auto-launch error: {}", e); - app_handle_clone.exit(1); - } - } - }); + // security: require an explicit --launch flag for silent auto-launch. + // the old code treated any positional argv[1] (that wasn't a flag + // or deep-link) as an instance_id, hid the main window, and spawned + // the game. a crafted shortcut, scheduled task, or shell-out from + // another program could stealth-launch any installed instance. + // (LCEL-06) + let mut launch_instance: Option = None; + let mut i = 1; + while i < args.len() { + let a = &args[i]; + if a == "--launch" && i + 1 < args.len() { + launch_instance = Some(args[i + 1].clone()); + i += 2; + continue; } + i += 1; + } + if let Some(instance_id) = launch_instance { + let app_handle_clone = app.handle().clone(); + tauri::async_runtime::spawn(async move { + if let Some(window) = app_handle_clone.get_webview_window("main") { + let _ = window.hide(); + } + let state = app_handle_clone.state::(); + match game::launch_game(app_handle_clone.clone(), state, instance_id, Vec::new(), vec![]).await { + Ok(_) => app_handle_clone.exit(0), + Err(e) => { + let _ = app_handle_clone.emit("backend-error", format!("Auto-launch: {e}")); + eprintln!("Auto-launch error: {}", e); + app_handle_clone.exit(1); + } + } + }); } Ok(()) }) diff --git a/src-tauri/src/networking/relay.rs b/src-tauri/src/networking/relay.rs index f4773c5..878010f 100644 --- a/src-tauri/src/networking/relay.rs +++ b/src-tauri/src/networking/relay.rs @@ -6,6 +6,10 @@ use tokio::time::sleep; use tokio_util::sync::CancellationToken; use crate::state::ProxyGuard; const PROXY_ADDR: &str = "proxy.mclegacyedition.xyz:2052"; //neo: yeah bro im hardcoding it +// security: cap read_line at 8 KiB. relay control lines are tiny (HOST/JOIN +// tokens + small id). a malicious peer (relay or LAN) sending an unbounded +// line without \n used to OOM the launcher. (LCEL-04) +const MAX_RELAY_LINE_BYTES: usize = 8 * 1024; async fn read_line(stream: &mut TcpStream) -> Result { let mut buf = Vec::new(); let mut byte = [0u8; 1]; @@ -13,6 +17,9 @@ async fn read_line(stream: &mut TcpStream) -> Result { stream.read_exact(&mut byte).await.map_err(|e| e.to_string())?; if byte[0] == b'\n' { break; } if byte[0] != b'\r' { buf.push(byte[0]); } + if buf.len() > MAX_RELAY_LINE_BYTES { + return Err(format!("relay line exceeded {} bytes", MAX_RELAY_LINE_BYTES)); + } } String::from_utf8(buf).map_err(|e| e.to_string()) } @@ -111,7 +118,11 @@ async fn run_relay_proxy( .map_err(|e| format!("Proxy connect failed: {}", e))?; write_line(&mut stream, &format!("JOIN {} 0 {}", auth_token, target_session)).await?; - let listener = TcpListener::bind("0.0.0.0:61000") + // security: bind to loopback only. the game process connects on + // 127.0.0.1:61000; binding 0.0.0.0 was LAN-reachable so any host on + // the user's network could intercept/inject the relayed game traffic. + // (LCEL-04) + let listener = TcpListener::bind("127.0.0.1:61000") .await .map_err(|e| format!("Bind failed: {}", e))?; let local_port = listener.local_addr().map_err(|e| e.to_string())?.port(); diff --git a/src-tauri/src/workshop_server.rs b/src-tauri/src/workshop_server.rs index c381c53..df76fd8 100644 --- a/src-tauri/src/workshop_server.rs +++ b/src-tauri/src/workshop_server.rs @@ -92,7 +92,10 @@ async fn handle(stream: tokio::net::TcpStream) { "application/octet-stream" }; let response = format!( - "HTTP/1.1 200 OK\r\nContent-Type: {}\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: *\r\nConnection: close\r\n\r\n", + // security: scope CORS to the tauri webview origin. was + // "*" which let any website the user visits fetch the + // workshop server. (LCEL-07) + "HTTP/1.1 200 OK\r\nContent-Type: {}\r\nContent-Length: {}\r\nAccess-Control-Allow-Origin: http://localhost:1420\r\nConnection: close\r\n\r\n", content_type, body.len(), ); diff --git a/src/services/LceOnlineService.ts b/src/services/LceOnlineService.ts index 6a2d3c3..4ea3531 100644 --- a/src/services/LceOnlineService.ts +++ b/src/services/LceOnlineService.ts @@ -1,3 +1,9 @@ +// security: the access token currently lives in localStorage under +// SESSION_KEY, which is readable by any JS in the webview origin +// (XSS exposure). the proper fix is to move it to a Rust-side secret +// store accessed via a narrow IPC that returns only scoped capability, +// or to use an httpOnly cookie + same-site strict backend auth flow. +// (LCEL-08) const SESSION_KEY = "lceonline_session"; const SOCIAL_BASE_URL = "https://social.mclegacyedition.xyz"; const AUTH_BASE_URL = "https://auth.mclegacyedition.xyz"; //neo: yeah bro im hardcoding all three