From c654a6d17eb58adbb15b9c47ab32bcc3a1666307 Mon Sep 17 00:00:00 2001 From: salmonik 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/sanitize.rs | 221 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 221 insertions(+) create mode 100644 src/sanitize.rs (limited to 'src/sanitize.rs') diff --git a/src/sanitize.rs b/src/sanitize.rs new file mode 100644 index 0000000..f251131 --- /dev/null +++ b/src/sanitize.rs @@ -0,0 +1,221 @@ +//! Input sanitization and validation utilities. +//! +//! Centralises all user-input validation to avoid path traversal, +//! command injection, and property injection vulnerabilities. + +use std::path::{Path, PathBuf}; + +/// The maximum length (in bytes) for an instance name. +const INSTANCE_NAME_MAX_BYTES: usize = 64; + +/// The maximum length (in bytes) for a seed value. +const SEED_MAX_BYTES: usize = 64; + +/// The maximum length (in bytes) for a Minecraft username. +const USERNAME_MAX_BYTES: usize = 16; + +/// Allowed characters for an instance name. +/// We restrict to a safe subset: alphanumeric, hyphens, underscores, dots. +/// This prevents path traversal via `..` or `/` and avoids filesystem issues. +fn instance_name_char_allowed(c: char) -> bool { + c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.' +} + +/// Validate and sanitise an instance name. +/// +/// Returns `None` if the name is invalid; otherwise returns the sanitised name +/// (trimmed, collapsed whitespace removed). +pub fn sanitize_instance_name(raw: &str) -> Option { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return None; + } + // Reject any name containing `..` — path traversal attempt + if trimmed.contains("..") { + return None; + } + // Reject names that would be filesystem metacharacters + if trimmed.contains('/') || trimmed.contains('\\') || trimmed.contains('\0') { + return None; + } + // Length limit + if trimmed.len() > INSTANCE_NAME_MAX_BYTES { + return None; + } + // Only allow safe characters + if !trimmed.chars().all(instance_name_char_allowed) { + return None; + } + // Must not start with a dot (hidden file issues) + if trimmed.starts_with('.') { + return None; + } + // Must not be a reserved name + match trimmed.to_ascii_lowercase().as_str() { + "" | "." | ".." | "con" | "prn" | "aux" | "nul" | "com1" | "com2" | "com3" | "com4" + | "lpt1" | "lpt2" | "lpt3" => return None, + _ => {} + } + Some(trimmed.to_string()) +} + +/// Sanitise a world-seed value to prevent Java Properties injection. +/// +/// Strips newlines, carriage returns, `=`, and `:` characters that could +/// inject additional properties or break the properties file format. +pub fn sanitize_seed(raw: &str) -> String { + let s: String = raw + .chars() + .filter(|c| !matches!(c, '\n' | '\r' | '=' | ':' | '\0')) + .take(SEED_MAX_BYTES) + .collect(); + s.trim().to_string() +} + +/// Validate a Minecraft username for RCON commands. +/// +/// Minecraft usernames are 3–16 characters, alphanumeric and underscores. +/// Returns `true` if the username is valid. +pub fn validate_username(raw: &str) -> bool { + let trimmed = raw.trim(); + if trimmed.len() < 3 || trimmed.len() > USERNAME_MAX_BYTES { + return false; + } + trimmed + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_') +} + +/// Verify that a resolved (canonical) path is within the given base directory. +/// +/// Both paths should be canonicalised before calling. Returns `true` if +/// `path` is a descendant of `base`. +#[allow(dead_code)] +pub fn path_is_within_base(path: &Path, base: &Path) -> bool { + path.canonicalize() + .ok() + .and_then(|p| base.canonicalize().ok().map(|b| p.starts_with(&b))) + .unwrap_or(false) +} + +/// Build a safe instance data directory path. +/// +/// The instance name is validated first. Returns `None` if the name is invalid. +#[allow(dead_code)] +pub fn instance_data_dir(server_dir: &Path, instance_name: &str) -> Option { + let safe = sanitize_instance_name(instance_name)?; + let dir = server_dir.join("instances").join(&safe).join("data"); + Some(dir) +} + +/// Build a safe instance mods directory path. +#[allow(dead_code)] +pub fn instance_mods_dir(server_dir: &Path, instance_name: &str) -> Option { + let data = instance_data_dir(server_dir, instance_name)?; + Some(data.join("mods")) +} + +/// Build a safe instance world directory path. +#[allow(dead_code)] +pub fn instance_world_dir(server_dir: &Path, instance_name: &str) -> Option { + let data = instance_data_dir(server_dir, instance_name)?; + Some(data.join("world")) +} + +/// Build a safe server.properties path for an instance. +#[allow(dead_code)] +pub fn instance_properties_path(server_dir: &Path, instance_name: &str) -> Option { + let data = instance_data_dir(server_dir, instance_name)?; + Some(data.join("server.properties")) +} + +/// Build a safe instance directory (parent of data/) for an instance. +#[allow(dead_code)] +pub fn instance_dir(server_dir: &Path, instance_name: &str) -> Option { + let safe = sanitize_instance_name(instance_name)?; + Some(server_dir.join("instances").join(&safe)) +} + +/// Validate that a backup filename does not contain path traversal. +pub fn sanitize_backup_filename(raw: &str) -> Option { + let trimmed = raw.trim(); + if trimmed.is_empty() { + return None; + } + if trimmed.contains("..") + || trimmed.contains('/') + || trimmed.contains('\\') + || trimmed.contains('\0') + { + return None; + } + if trimmed.len() > 256 { + return None; + } + Some(trimmed.to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_sanitize_instance_name_ok() { + assert_eq!(sanitize_instance_name("my-world").unwrap(), "my-world"); + assert_eq!(sanitize_instance_name("test_123").unwrap(), "test_123"); + assert_eq!(sanitize_instance_name("V1.20.4").unwrap(), "V1.20.4"); + assert_eq!(sanitize_instance_name(" hello ").unwrap(), "hello"); + } + + #[test] + fn test_sanitize_instance_name_rejected() { + assert!(sanitize_instance_name("").is_none()); + assert!(sanitize_instance_name(" ").is_none()); + assert!(sanitize_instance_name("a/b").is_none()); + assert!(sanitize_instance_name("a\\b").is_none()); + assert!(sanitize_instance_name("..").is_none()); + assert!(sanitize_instance_name("foo/../bar").is_none()); + assert!(sanitize_instance_name(".hidden").is_none()); + assert!(sanitize_instance_name("hello world").is_none()); // space not allowed + assert!(sanitize_instance_name("a@b").is_none()); + } + + #[test] + fn test_sanitize_seed() { + assert_eq!(sanitize_seed("12345"), "12345"); + assert_eq!(sanitize_seed("hello\nworld"), "helloworld"); + assert_eq!(sanitize_seed("key=value"), "keyvalue"); + // The seed is capped at SEED_MAX_BYTES=64. The string below is 64 chars after sanitization. + let long_seed = "-123456789012345678901234567890123456789012345678901234567890123"; + assert_eq!(long_seed.len(), 64); + assert_eq!( + sanitize_seed( + "-1234567890123456789012345678901234567890123456789012345678901234567890" + ), + long_seed + ); + } + + #[test] + fn test_validate_username() { + assert!(validate_username("Notch")); + assert!(validate_username("player_123")); + assert!(!validate_username("ab")); // too short + assert!(!validate_username("a@b.com")); // special chars + assert!(!validate_username("")); // empty + } + + #[test] + fn test_path_is_within_base() { + use std::fs; + let dir = tempfile::tempdir().unwrap(); + let base = dir.path().join("server"); + fs::create_dir_all(&base).unwrap(); + let sub = base.join("instances").join("test").join("data"); + fs::create_dir_all(&sub).unwrap(); + assert!(path_is_within_base(&sub, &base)); + // Path outside base should fail + let outside = dir.path().join("other"); + assert!(!path_is_within_base(&outside, &base)); + } +} -- cgit v1.2.3