fix(LCEL-05): allowlist extra_args forwarded to game

This commit is contained in:
M1noa 2026-07-16 20:27:34 -05:00
parent 543a5890c3
commit 080a8a07ea
5 changed files with 190 additions and 3 deletions

View file

@ -40,12 +40,47 @@ pub fn save_file_dialog(title: String, filename: String, filters: Vec<String>) -
}
}
// 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<u8>) -> Result<(), String> {
pub fn write_binary_file(
app: tauri::AppHandle,
path: String,
data: Vec<u8>,
) -> 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<Vec<u8>, String> {
pub fn read_binary_file(
app: tauri::AppHandle,
path: String,
) -> Result<Vec<u8>, 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())
}

View file

@ -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<String> {
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<McServer>,
extra_args: Vec<String>,
) -> 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());

View file

@ -14,7 +14,30 @@ pub fn get_plugins_dir(app: tauri::AppHandle) -> Result<String, String> {
}
#[tauri::command]
pub fn list_directory(path: String) -> Result<Vec<DirEntry>, String> {
pub fn list_directory(
app: tauri::AppHandle,
path: String,
) -> Result<Vec<DirEntry>, 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 {

View file

@ -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<String>,
headers: std::collections::HashMap<String, String>,
) -> Result<HttpResponse, String> {
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),

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