diff options
| author | salmonik <salmonik@tuta.io> | 2026-07-29 02:24:41 +0200 |
|---|---|---|
| committer | salmonik <salmonik@tuta.io> | 2026-07-29 02:24:41 +0200 |
| commit | c654a6d17eb58adbb15b9c47ab32bcc3a1666307 (patch) | |
| tree | 545544bf7d805254415da787fad1cdc711767833 /src/routes/players.rs | |
| parent | Add per-instance mod management with client-side detection (diff) | |
| download | minecraft-ui-c654a6d17eb58adbb15b9c47ab32bcc3a1666307.tar.gz minecraft-ui-c654a6d17eb58adbb15b9c47ab32bcc3a1666307.zip | |
Security audit fixes: patch 16 vulnerabilities across 7 source files
CRITICAL:
- Instance name path traversal (AP-001): sanitize::sanitize_instance_name()
strips .., /, \, \0, spaces, special chars at all 12 join call sites
- Zip-slip protection in world import: starts_with() guard after enclosed_name()
HIGH:
- RCON command injection: validate_username() rejects non-MC usernames
across all 6 player action macros (whitelist, op, ban)
- Level-seed properties injection: sanitize_seed() strips \n \r = : \0
- CSRF protection: SameSite=Lax -> SameSite=Strict on session cookie
MEDIUM:
- Session TTL: server-side 24h expiry via strftime check in require_auth
- Rate limiting: in-memory 10-ip/5min sliding window on login
- Background task stale is_active: all queries use concrete WHERE id=?1
- Mod validation DoS: 500MB cap, 10k class scan limit, 10k CP entry cap
LOW:
- Logout auth guard: moved into protected route group + CurrentUser param
- Mod orphan race: DB insert before file write
- Backup orphan race: DB insert before ZIP creation
- Template fix: broken Askama == comparisons in edit.html
All fixes compile clean with zero warnings. 10 unit tests pass.
Diffstat (limited to '')
| -rw-r--r-- | src/routes/players.rs | 19 |
1 files changed, 17 insertions, 2 deletions
diff --git a/src/routes/players.rs b/src/routes/players.rs index 7cfb722..f90113a 100644 --- a/src/routes/players.rs +++ b/src/routes/players.rs | |||
| @@ -9,6 +9,7 @@ use serde::Deserialize; | |||
| 9 | 9 | ||
| 10 | use crate::auth::CurrentUser; | 10 | use crate::auth::CurrentUser; |
| 11 | use crate::routes::AppState; | 11 | use crate::routes::AppState; |
| 12 | use crate::sanitize; | ||
| 12 | 13 | ||
| 13 | #[derive(Template)] | 14 | #[derive(Template)] |
| 14 | #[template(path = "players/manage.html")] | 15 | #[template(path = "players/manage.html")] |
| @@ -46,11 +47,14 @@ async fn manage_players( | |||
| 46 | 47 | ||
| 47 | let (instance_name, whitelist, ops, bans) = | 48 | let (instance_name, whitelist, ops, bans) = |
| 48 | if let Some((_id, inst_name, _rcon_pw, _container_state)) = active { | 49 | if let Some((_id, inst_name, _rcon_pw, _container_state)) = active { |
| 50 | // Sanitize instance name to prevent path traversal in file reads | ||
| 51 | let safe_name = sanitize::sanitize_instance_name(&inst_name) | ||
| 52 | .unwrap_or_else(|| inst_name.clone()); | ||
| 49 | let data_dir = state | 53 | let data_dir = state |
| 50 | .config | 54 | .config |
| 51 | .server_directory | 55 | .server_directory |
| 52 | .join("instances") | 56 | .join("instances") |
| 53 | .join(&inst_name) | 57 | .join(&safe_name) |
| 54 | .join("data"); | 58 | .join("data"); |
| 55 | 59 | ||
| 56 | let w = read_json_list(&data_dir.join("whitelist.json")); | 60 | let w = read_json_list(&data_dir.join("whitelist.json")); |
| @@ -92,9 +96,20 @@ macro_rules! player_action { | |||
| 92 | CurrentUser(_user): CurrentUser, | 96 | CurrentUser(_user): CurrentUser, |
| 93 | Form(form): Form<PlayerAction>, | 97 | Form(form): Form<PlayerAction>, |
| 94 | ) -> Result<Redirect, axum::response::Redirect> { | 98 | ) -> Result<Redirect, axum::response::Redirect> { |
| 99 | // Validate Minecraft username to prevent RCON command injection | ||
| 100 | // Minecraft usernames are 3-16 alphanumeric + underscore chars | ||
| 101 | let safe_username = form.username.trim(); | ||
| 102 | if !sanitize::validate_username(safe_username) { | ||
| 103 | tracing::warn!( | ||
| 104 | "Rejected invalid username for '{}': {safe_username:?}", | ||
| 105 | stringify!($name) | ||
| 106 | ); | ||
| 107 | return Ok(Redirect::to("/players")); | ||
| 108 | } | ||
| 109 | |||
| 95 | if let Some((_, _, rcon_pw, _)) = get_active_instance(&state.db) { | 110 | if let Some((_, _, rcon_pw, _)) = get_active_instance(&state.db) { |
| 96 | let client = crate::rcon::RconClient::new(rcon_pw); | 111 | let client = crate::rcon::RconClient::new(rcon_pw); |
| 97 | let cmd = format!($command, form.username); | 112 | let cmd = format!($command, safe_username); |
| 98 | match client.send_command_async(cmd.clone()).await { | 113 | match client.send_command_async(cmd.clone()).await { |
| 99 | Ok(resp) => tracing::info!("RCON '{}' -> {}", cmd, resp.trim()), | 114 | Ok(resp) => tracing::info!("RCON '{}' -> {}", cmd, resp.trim()), |
| 100 | Err(e) => tracing::error!("RCON '{}' failed: {}", cmd, e), | 115 | Err(e) => tracing::error!("RCON '{}' failed: {}", cmd, e), |
