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/backups.rs | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

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

diff --git a/src/routes/backups.rs b/src/routes/backups.rs
index d68be8d..b55d5c8 100644
--- a/src/routes/backups.rs
+++ b/src/routes/backups.rs
@@ -12,6 +12,7 @@ use zip::write::FileOptions;
 
 use crate::auth::CurrentUser;
 use crate::routes::AppState;
+use crate::sanitize;
 
 #[derive(Template)]
 #[template(path = "backups/list.html")]
@@ -97,20 +98,37 @@ async fn create_backup(
         .map_err(|_| Redirect::to("/instances"))?;
     drop(conn);
 
+    // Sanitize instance name to prevent path traversal
+    let safe_name = match sanitize::sanitize_instance_name(&instance_name) {
+        Some(n) => n,
+        None => return Ok(Redirect::to(&format!("/instances/{instance_id}/backups"))),
+    };
+
     let timestamp = chrono::Utc::now().format("%Y%m%d_%H%M%S");
-    let filename = format!("{instance_name}_{timestamp}.zip");
+    let filename = format!("{safe_name}_{timestamp}.zip");
 
     let data_dir = state
         .config
         .server_directory
         .join("instances")
-        .join(&instance_name)
+        .join(&safe_name)
         .join("data");
 
     let backup_dir = state.config.server_directory.join("backups");
     let _ = std::fs::create_dir_all(&backup_dir);
     let backup_path = backup_dir.join(&filename);
 
+    // FIX: Insert DB record BEFORE creating the ZIP file.
+    // This prevents orphaned backup files if the process crashes between ZIP creation and DB insert.
+    let conn = state.db.conn();
+    conn.execute(
+        "INSERT INTO backups (instance_id, filename) VALUES (?1, ?2)",
+        params![instance_id, filename],
+    )
+    .ok();
+    drop(conn);
+
+    // Now create the ZIP file (DB insert already succeeded)
     if let Ok(file) = std::fs::File::create(&backup_path) {
         let mut zip = zip::ZipWriter::new(file);
         let options: FileOptions<'_, ()> =
@@ -130,14 +148,6 @@ async fn create_backup(
         let _ = zip.finish();
     }
 
-    let conn = state.db.conn();
-    conn.execute(
-        "INSERT INTO backups (instance_id, filename) VALUES (?1, ?2)",
-        params![instance_id, filename],
-    )
-    .ok();
-    drop(conn);
-
     Ok(Redirect::to(&format!("/instances/{instance_id}/backups")))
 }
 
@@ -174,11 +184,17 @@ async fn download_backup(
         .map_err(|_| (StatusCode::NOT_FOUND, "Backup not found"))?;
     drop(conn);
 
+    // Sanitize backup filename to prevent path traversal
+    let safe_filename = match sanitize::sanitize_backup_filename(&filename) {
+        Some(f) => f,
+        None => return Err((StatusCode::BAD_REQUEST, "Invalid backup filename")),
+    };
+
     let backup_path = state
         .config
         .server_directory
         .join("backups")
-        .join(&filename);
+        .join(&safe_filename);
 
     let content =
         std::fs::read(&backup_path).map_err(|_| (StatusCode::NOT_FOUND, "File not found"))?;
@@ -187,7 +203,7 @@ async fn download_backup(
         .header(header::CONTENT_TYPE, "application/zip")
         .header(
             header::CONTENT_DISPOSITION,
-            format!("attachment; filename=\"{filename}\""),
+            format!("attachment; filename=\"{safe_filename}\""),
         )
         .body(Body::from(content))
         .unwrap())
-- 
cgit v1.2.3