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. --- .../source-sink-flows-all-severities.md | 143 +++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 piolium/attack-surface/source-sink-flows-all-severities.md (limited to 'piolium/attack-surface/source-sink-flows-all-severities.md') diff --git a/piolium/attack-surface/source-sink-flows-all-severities.md b/piolium/attack-surface/source-sink-flows-all-severities.md new file mode 100644 index 0000000..d7f93da --- /dev/null +++ b/piolium/attack-surface/source-sink-flows-all-severities.md @@ -0,0 +1,143 @@ +# Source-to-Sink Flow Paths (All Severities) + +Generated by manual structural extraction (codeql not available on this system). + +## Critical Flows + +### Flow C1: Instance Name Path Traversal → Arbitrary Directory Creation +- **Source:** `POST /instances/new` → `CreateInstanceForm.name` (free text, no validation) +- **Path:** `src/routes/instances.rs:165` → `form.name` → `src/routes/instances.rs:205` `.join(&form.name)` → `std::fs::create_dir_all()` +- **Risk:** Attacker can create directories anywhere the webUI user has write access via `../` in instance name +- **Control:** NO sanitization whatsoever on `form.name` +- **CWE:** CWE-22 (Path Traversal) + +### Flow C2: Instance Name Path Traversal → Arbitrary Directory Deletion +- **Source:** `POST /instances/{id}/delete` → name from DB (originally `form.name`, no sanitization) +- **Path:** `src/routes/instances.rs:360` → `DELETE FROM instances WHERE id=?` → name from DB → `src/routes/instances.rs:392-393` `.join(&name)` → `std::fs::remove_dir_all()` +- **Risk:** Attacker can delete directories anywhere via `../` in instance name +- **Control:** Name stored in DB at creation time without sanitization +- **CWE:** CWE-22 (Path Traversal) + +## High Severity Flows + +### Flow H1: RCON Command Injection via Player Username +- **Source:** `POST /players/whitelist/add` → `PlayerAction.username` (free text) +- **Path:** `src/routes/players.rs:93` → `form.username` → `src/routes/players.rs:97` `format!("whitelist add {}", form.username)` → `RconClient.send_command_async(cmd)` +- **Risk:** Attacker can inject arbitrary RCON commands via username field (e.g., `foo; op attacker`) +- **Control:** NO sanitization on `form.username` +- **CWE:** CWE-77 (Command Injection), CWE-88 (Argument Injection) + +### Flow H2: RCON Command Injection via OP/Ban Username +- **Source:** `POST /players/op/add` → `PlayerAction.username` (same for ban/pardon/deop) +- **Path:** `src/routes/players.rs:90-98` → `player_action!` macro generates: `format!("op {}", form.username)` → `RconClient.send_command_async()` +- **Risk:** Same as H1 — arbitrary RCON command injection +- **CWE:** CWE-77, CWE-88 + +### Flow H3: No Session Expiration/TTL Server-Side +- **Source:** Any authenticated session +- **Path:** Session stored in SQLite with `created_at` timestamp but **never checked** on authentication +- **Risk:** Stolen session cookies remain valid indefinitely server-side +- **CWE:** CWE-613 (Insufficient Session Expiration) + +### Flow H4: No CSRF Protection on Any POST Endpoint +- **Source:** All POST endpoints +- **Path:** No CSRF token validation anywhere in the application +- **Risk:** Cross-site request forgery — any website can trigger state-changing operations +- **CWE:** CWE-352 (Cross-Site Request Forgery) + +## Medium Severity Flows + +### Flow M1: Mod Filename After Directory Stripping Special Characters +- **Source:** `POST /instances/{id}/mods` → multipart upload filename +- **Path:** `field.file_name()` → `process_jar_upload()` → `Path::new(filename).file_name()` (strips dir) → `mods_dir.join(&safe_name)` → `std::fs::write()` +- **Risk:** After directory stripping, filename may still contain special characters or Unicode normalization issues +- **CWE:** CWE-22 + +### Flow M2: World Import ZIP Extraction (Mitigated) +- **Source:** `POST /instances/{id}/world/import` → multipart ZIP +- **Path:** `zip::ZipArchive::new()` → `archive.by_index(i)` → `file.enclosed_name()` → `world_dir.join(p)` → `File::create()` +- **Risk:** Zip slip partially mitigated by `enclosed_name()` but edge cases may exist +- **CWE:** CWE-22 + +### Flow M3: Backup Download Using Filename from DB +- **Source:** `GET /backups/{id}/download` → `backup_id` path parameter +- **Path:** `Path(backup_id)` → SQL SELECT filename → `backup_dir.join(&filename)` → `std::fs::read()` +- **Risk:** If filename field in DB is manipulated, could read arbitrary files +- **CWE:** CWE-22 + +### Flow M4: Docker Container Environment Variable Injection +- **Source:** `POST /instances/new` → various form fields +- **Path:** `CreateInstanceForm` → SQL INSERT → Instance model → `docker.start_container(&instance)` → `ContainerConfig.env` includes instance fields unescaped +- **Risk:** User-controlled strings passed to Docker container environment +- **CWE:** CWE-77 + +### Flow M5: Level-Seed Injection into server.properties +- **Source:** `POST /instances/{id}/world/regenerate` → `form.seed` +- **Path:** `form.seed` → `format!("level-seed={seed}")` → `std::fs::write(&props_path, ...)` +- **Risk:** Seed string written directly to properties file; could corrupt configuration +- **CWE:** CWE-93 (CRLF Injection) + +### Flow M6: Weak RCON Password Generation +- **Source:** `generate_rcon_password()` called during instance creation +- **Path:** `rand::thread_rng()` → 16-char alphanumeric → stored in DB +- **Risk:** Not cryptographically secure RNG for password generation +- **CWE:** CWE-338 (Use of Cryptographically Weak PRNG) + +### Flow M7: RCON Password Exposed to All Authenticated Users +- **Source:** `GET /instances/{id}/edit` template +- **Path:** SQL SELECT includes `rcon_password` → `EditInstanceTemplate` → rendered in HTML +- **Risk:** Any authenticated user can see the RCON password +- **CWE:** CWE-200 (Information Exposure) + +### Flow M8: Logout Handler Without Auth Middleware +- **Source:** `POST /logout` (registered on outer router, not behind `require_auth`) +- **Path:** Cookie header parsed → `DELETE FROM sessions WHERE id = ?` +- **Risk:** Anonymous requests can delete sessions if session ID is known/guessed +- **CWE:** CWE-306 (Missing Authentication) + +### Flow M9: No Rate Limiting on Login +- **Source:** `POST /login` +- **Path:** Infinite brute-force attempts possible +- **Risk:** Account enumeration and password brute-force +- **CWE:** CWE-307 (Improper Restriction of Excessive Authentication Attempts) + +### Flow M10: Cookie Without Secure Flag +- **Source:** Login sets cookie +- **Path:** `format!("{SESSION_COOKIE}={session_id}; Path=/; HttpOnly; Max-Age=86400; SameSite=Lax")` +- **Risk:** Cookie transmitted over HTTP (missing Secure flag) +- **CWE:** CWE-614 (Sensitive Cookie in HTTPS Without Secure Attribute) + +## Low Severity Flows + +### Flow L1: Mod JAR Parsing Denial of Service +- **Source:** `POST /instances/{id}/mods` → uploaded JAR +- **Path:** `validate_mod()` → `ZipArchive::new()` → `scan_jar_for_side()` → constant pool parsing +- **Risk:** Crafted JAR with malicious constant pool could cause OOM or infinite loop +- **CWE:** CWE-770 (Allocation of Resources Without Limits) + +### Flow L2: Recursive ZIP Extraction Without Depth Limit +- **Source:** `POST /instances/{id}/mods` → ZIP containing JAR (which is also a ZIP) +- **Path:** `process_zip_upload()` → extracts JAR → `process_jar_upload()` → `validate_mod()` opens JAR as ZIP again +- **Risk:** No depth counter; nested ZIP-in-JAR could cause resource exhaustion +- **CWE:** CWE-674 (Uncontrolled Recursion) + +### Flow L3: Backup Restore is No-Op +- **Source:** `POST /backups/{id}/restore` +- **Path:** Looks up (instance_id, _filename) from DB, then returns redirect without restoring +- **Risk:** Backup restore functionality is implemented as a stub; users think data is restored +- **CWE:** CWE-1039 (Automated Recognition Mechanism with Inadequate Detection) + +### Flow L4: No Audit Logging +- **Source:** All state-changing operations +- **Path:** No logging of who performed what action +- **Risk:** No accountability or forensic evidence for security incidents + +### Flow L5: ThreadRng Used for Password Generation +- **Source:** `src/routes/instances.rs:530` +- **Path:** `rand::thread_rng()` → 36^16 password space +- **Risk:** Predictable RNG seed may make RCON password guessable + +### Flow L6: Cookie Splitting Parsing is Naive +- **Source:** `src/auth.rs:107` and `src/auth.rs:182` +- **Path:** `raw_cookies.split(';')` then `split_once('=')` +- **Risk:** If cookie value contains `=` or `;`, parsing may be confused -- cgit v1.2.3