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/mods.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/mods.rs | 54 |
1 files changed, 41 insertions, 13 deletions
diff --git a/src/routes/mods.rs b/src/routes/mods.rs index 4716fec..d8f0845 100644 --- a/src/routes/mods.rs +++ b/src/routes/mods.rs | |||
| @@ -13,6 +13,7 @@ use std::io::{Cursor, Read}; | |||
| 13 | use crate::auth::CurrentUser; | 13 | use crate::auth::CurrentUser; |
| 14 | use crate::mods; | 14 | use crate::mods; |
| 15 | use crate::routes::AppState; | 15 | use crate::routes::AppState; |
| 16 | use crate::sanitize; | ||
| 16 | 17 | ||
| 17 | #[derive(Template)] | 18 | #[derive(Template)] |
| 18 | #[template(path = "mods/list.html")] | 19 | #[template(path = "mods/list.html")] |
| @@ -119,11 +120,17 @@ async fn upload_mod( | |||
| 119 | None => return Err(Redirect::to("/instances")), | 120 | None => return Err(Redirect::to("/instances")), |
| 120 | }; | 121 | }; |
| 121 | 122 | ||
| 123 | // Sanitize instance name to prevent path traversal | ||
| 124 | let safe_name = match sanitize::sanitize_instance_name(&instance_name) { | ||
| 125 | Some(n) => n, | ||
| 126 | None => return Err(Redirect::to("/instances")), | ||
| 127 | }; | ||
| 128 | |||
| 122 | let mods_dir = state | 129 | let mods_dir = state |
| 123 | .config | 130 | .config |
| 124 | .server_directory | 131 | .server_directory |
| 125 | .join("instances") | 132 | .join("instances") |
| 126 | .join(&instance_name) | 133 | .join(&safe_name) |
| 127 | .join("data") | 134 | .join("data") |
| 128 | .join("mods"); | 135 | .join("mods"); |
| 129 | let _ = std::fs::create_dir_all(&mods_dir); | 136 | let _ = std::fs::create_dir_all(&mods_dir); |
| @@ -243,15 +250,8 @@ fn process_jar_upload( | |||
| 243 | result.side | 250 | result.side |
| 244 | ); | 251 | ); |
| 245 | 252 | ||
| 246 | if result.accepted { | 253 | // FIX: Insert DB record BEFORE writing file to disk. |
| 247 | let dest = mods_dir.join(&safe_name); | 254 | // This prevents orphaned mod files if the process crashes between write and DB insert. |
| 248 | if std::fs::write(&dest, bytes).is_ok() { | ||
| 249 | tracing::info!("Written to disk: {}", dest.display()); | ||
| 250 | } else { | ||
| 251 | tracing::error!("Failed to write {}", dest.display()); | ||
| 252 | } | ||
| 253 | } | ||
| 254 | |||
| 255 | let conn = state.db.conn(); | 255 | let conn = state.db.conn(); |
| 256 | if let Err(e) = conn.execute( | 256 | if let Err(e) = conn.execute( |
| 257 | "INSERT INTO mods (instance_id, filename, display_name, side, accepted, rejection_reason) | 257 | "INSERT INTO mods (instance_id, filename, display_name, side, accepted, rejection_reason) |
| @@ -266,10 +266,20 @@ fn process_jar_upload( | |||
| 266 | ], | 266 | ], |
| 267 | ) { | 267 | ) { |
| 268 | tracing::error!("DB insert failed: {e}"); | 268 | tracing::error!("DB insert failed: {e}"); |
| 269 | drop(conn); | ||
| 270 | return; | ||
| 269 | } | 271 | } |
| 270 | drop(conn); | 272 | drop(conn); |
| 271 | 273 | ||
| 274 | // Now write to disk (DB insert succeeded, so on restart we can detect incomplete writes) | ||
| 272 | if result.accepted { | 275 | if result.accepted { |
| 276 | let dest = mods_dir.join(&safe_name); | ||
| 277 | if std::fs::write(&dest, bytes).is_ok() { | ||
| 278 | tracing::info!("Written to disk: {}", dest.display()); | ||
| 279 | } else { | ||
| 280 | tracing::error!("Failed to write {}", dest.display()); | ||
| 281 | } | ||
| 282 | |||
| 273 | let conn = state.db.conn(); | 283 | let conn = state.db.conn(); |
| 274 | let _ = conn.execute( | 284 | let _ = conn.execute( |
| 275 | "UPDATE instances SET has_modpack = 1 WHERE id = ?1", | 285 | "UPDATE instances SET has_modpack = 1 WHERE id = ?1", |
| @@ -336,11 +346,17 @@ async fn rescan_mods( | |||
| 336 | None => return Err(Redirect::to("/instances")), | 346 | None => return Err(Redirect::to("/instances")), |
| 337 | }; | 347 | }; |
| 338 | 348 | ||
| 349 | // Sanitize instance name to prevent path traversal | ||
| 350 | let safe_name = match sanitize::sanitize_instance_name(&instance_name) { | ||
| 351 | Some(n) => n, | ||
| 352 | None => return Err(Redirect::to("/instances")), | ||
| 353 | }; | ||
| 354 | |||
| 339 | let mods_dir = state | 355 | let mods_dir = state |
| 340 | .config | 356 | .config |
| 341 | .server_directory | 357 | .server_directory |
| 342 | .join("instances") | 358 | .join("instances") |
| 343 | .join(&instance_name) | 359 | .join(&safe_name) |
| 344 | .join("data") | 360 | .join("data") |
| 345 | .join("mods"); | 361 | .join("mods"); |
| 346 | 362 | ||
| @@ -386,15 +402,27 @@ async fn delete_mod( | |||
| 386 | ) | 402 | ) |
| 387 | .map_err(|_| Redirect::to(&format!("/instances/{instance_id}/mods")))?; | 403 | .map_err(|_| Redirect::to(&format!("/instances/{instance_id}/mods")))?; |
| 388 | 404 | ||
| 405 | // Sanitize instance name to prevent path traversal | ||
| 406 | let safe_name = match sanitize::sanitize_instance_name(&instance_name) { | ||
| 407 | Some(n) => n, | ||
| 408 | None => return Err(Redirect::to(&format!("/instances/{instance_id}/mods"))), | ||
| 409 | }; | ||
| 410 | |||
| 411 | // Sanitize filename to prevent path traversal | ||
| 412 | let safe_filename = match sanitize::sanitize_backup_filename(&filename) { | ||
| 413 | Some(f) => f, | ||
| 414 | None => return Err(Redirect::to(&format!("/instances/{instance_id}/mods"))), | ||
| 415 | }; | ||
| 416 | |||
| 389 | // Delete file from disk | 417 | // Delete file from disk |
| 390 | let mod_path = state | 418 | let mod_path = state |
| 391 | .config | 419 | .config |
| 392 | .server_directory | 420 | .server_directory |
| 393 | .join("instances") | 421 | .join("instances") |
| 394 | .join(&instance_name) | 422 | .join(&safe_name) |
| 395 | .join("data") | 423 | .join("data") |
| 396 | .join("mods") | 424 | .join("mods") |
| 397 | .join(&filename); | 425 | .join(&safe_filename); |
| 398 | let _ = std::fs::remove_file(&mod_path); | 426 | let _ = std::fs::remove_file(&mod_path); |
| 399 | 427 | ||
| 400 | // Remove from DB | 428 | // Remove from DB |
