From c654a6d17eb58adbb15b9c47ab32bcc3a1666307 Mon Sep 17 00:00:00 2001
From: salmonik <salmonik@tuta.io>
Date: Wed, 29 Jul 2026 02:24:41 +0200
Subject: 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.
---
 src/routes/mods.rs | 54 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 13 deletions(-)

(limited to 'src/routes/mods.rs')

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};
 use crate::auth::CurrentUser;
 use crate::mods;
 use crate::routes::AppState;
+use crate::sanitize;
 
 #[derive(Template)]
 #[template(path = "mods/list.html")]
@@ -119,11 +120,17 @@ async fn upload_mod(
         None => return Err(Redirect::to("/instances")),
     };
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Err(Redirect::to("/instances")),
+    };
+
     let mods_dir = state
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data")
         .join("mods");
     let _ = std::fs::create_dir_all(&mods_dir);
@@ -243,15 +250,8 @@ fn process_jar_upload(
         result.side
     );
 
-    if result.accepted {
-        let dest = mods_dir.join(&safe_name);
-        if std::fs::write(&dest, bytes).is_ok() {
-            tracing::info!("Written to disk: {}", dest.display());
-        } else {
-            tracing::error!("Failed to write {}", dest.display());
-        }
-    }
-
+    // FIX: Insert DB record BEFORE writing file to disk.
+    // This prevents orphaned mod files if the process crashes between write and DB insert.
     let conn = state.db.conn();
     if let Err(e) = conn.execute(
         "INSERT INTO mods (instance_id, filename, display_name, side, accepted, rejection_reason)
@@ -266,10 +266,20 @@ fn process_jar_upload(
         ],
     ) {
         tracing::error!("DB insert failed: {e}");
+        drop(conn);
+        return;
     }
     drop(conn);
 
+    // Now write to disk (DB insert succeeded, so on restart we can detect incomplete writes)
     if result.accepted {
+        let dest = mods_dir.join(&safe_name);
+        if std::fs::write(&dest, bytes).is_ok() {
+            tracing::info!("Written to disk: {}", dest.display());
+        } else {
+            tracing::error!("Failed to write {}", dest.display());
+        }
+
         let conn = state.db.conn();
         let _ = conn.execute(
             "UPDATE instances SET has_modpack = 1 WHERE id = ?1",
@@ -336,11 +346,17 @@ async fn rescan_mods(
         None => return Err(Redirect::to("/instances")),
     };
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Err(Redirect::to("/instances")),
+    };
+
     let mods_dir = state
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data")
         .join("mods");
 
@@ -386,15 +402,27 @@ async fn delete_mod(
         )
         .map_err(|_| Redirect::to(&format!("/instances/{instance_id}/mods")))?;
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Err(Redirect::to(&format!("/instances/{instance_id}/mods"))),
+    };
+
+    // Sanitize filename to prevent path traversal
+    let safe_filename = match sanitize::sanitize_backup_filename(&filename) {
+        Some(f) => f,
+        None => return Err(Redirect::to(&format!("/instances/{instance_id}/mods"))),
+    };
+
     // Delete file from disk
     let mod_path = state
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data")
         .join("mods")
-        .join(&filename);
+        .join(&safe_filename);
     let _ = std::fs::remove_file(&mod_path);
 
     // Remove from DB
-- 
cgit v1.2.3