summaryrefslogtreecommitdiffstats
path: root/piolium/attack-surface/deep-probe-summary.md
diff options
context:
space:
mode:
authorsalmonik <salmonik@tuta.io>2026-07-29 02:24:41 +0200
committersalmonik <salmonik@tuta.io>2026-07-29 02:24:41 +0200
commitc654a6d17eb58adbb15b9c47ab32bcc3a1666307 (patch)
tree545544bf7d805254415da787fad1cdc711767833 /piolium/attack-surface/deep-probe-summary.md
parentAdd per-instance mod management with client-side detection (diff)
downloadminecraft-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 'piolium/attack-surface/deep-probe-summary.md')
-rw-r--r--piolium/attack-surface/deep-probe-summary.md164
1 files changed, 164 insertions, 0 deletions
diff --git a/piolium/attack-surface/deep-probe-summary.md b/piolium/attack-surface/deep-probe-summary.md
new file mode 100644
index 0000000..b3fb8df
--- /dev/null
+++ b/piolium/attack-surface/deep-probe-summary.md
@@ -0,0 +1,164 @@
1# Deep Probe Summary: salmon/minecraft-ui
2
3**Generated:** 2025-07-29
4**Phase:** P8 — Manual Attack Surface Probe
5**Status:** Complete
6**Loops:** 1
7**Total findings:** 10
8**CRITICAL:** 1 (P8-003)
9**HIGH:** 5 (P8-001, P8-002, P8-004, P8-006, P8-009)
10**MEDIUM:** 3 (P8-005, P8-007, P8-010)
11**LOW:** 1 (P8-008)
12
13---
14
15## Probe Methodology
16
17This manual deep probe conducted:
18
191. **Full source code review** of all 16 Rust source files (1,540+ lines analyzed)
202. **Attack surface inventory** — 38 HTTP routes catalogued with entry points, sinks, and trust boundaries
213. **Backward reasoning** — trace from sinks (file write, RCON command, Docker env, SQL) back to attacker inputs
224. **Contradiction reasoning** — identify gaps between intended security boundaries and actual code
235. **Cross-model analysis** — chain multiple low-severity issues into higher-severity attack paths
246. **Verification** — all findings verified with file:line code evidence via read/grep
25
26---
27
28## Finding Summary
29
30| ID | Title | Severity | Category | Attack Vector | Existing Draft |
31|----|-------|----------|----------|---------------|---------------|
32| P8-001 | MOTD Docker Environment Injection | **HIGH** | CWE-77, CWE-93 | Newline injection in MOTD overrides Docker container env vars | Enhanced from p4-016 |
33| P8-002 | RCON Command Injection | **HIGH** | CWE-77, CWE-88 | Unsanitized username in RCON command strings | Updated p4-002 |
34| P8-003 | Instance Name Path Traversal Chain | **CRITICAL** | CWE-22 | `..` in instance name enables arbitrary file write/read/delete | Expanded p4-001 |
35| P8-004 | Level-Seed Properties Injection | **HIGH** | CWE-93, CWE-88 | Seed with newlines injects arbitrary Java properties | Enhanced p4-020 |
36| P8-005 | No Server-Side Session Expiration | **MEDIUM** | CWE-613 | Stolen session cookies never expire | Confirmed p4-004 |
37| P8-006 | Missing CSRF Protection | **HIGH** | CWE-352 | All POST endpoints vulnerable to cross-site request forgery | Confirmed p4-003 |
38| P8-007 | POST /logout Missing Auth Guard | **MEDIUM** | CWE-306 | Logout handler not behind require_auth middleware | Confirmed p4-008 |
39| P8-008 | Weak RCON Password Generation | **LOW** | CWE-338 | thread_rng() used instead of OsRng | Confirmed p4-007 |
40| P8-009 | RCON Password Exposed to All Users | **HIGH** | CWE-200 | rcon_password in edit form for any authenticated user | Confirmed p4-006 |
41| P8-010 | Mod JAR Validation Denial of Service | **MEDIUM** | CWE-770, CWE-674 | Crafted JAR constant pool, no timeout, no recursion limit | Enhanced p4-014 |
42
43---
44
45## Key Findings — Detailed Analysis
46
47### P8-003: Instance Name Path Traversal Chain (CRITICAL)
48
49**The most severe finding.** The instance `name` field is used in 8 different filesystem operations without any sanitization. Rust's `Path::join()` follows `..` components, enabling:
50
51- **Arbitrary directory creation** via `create_dir_all` at instance creation
52- **Arbitrary file write** via mod upload (`fs::write`) and world import (`File::create`)
53- **Arbitrary directory deletion** via `remove_dir_all` at instance deletion
54- **Arbitrary file read** via world export and backup download
55
56The full attack chain:
571. Create instance with `name=../../../tmp/pwned`
582. Upload mod/world to write files at `/tmp/pwned/data/mods/evil.so`
593. Delete instance to remove `/tmp/pwned` directory tree
60
61**Code path:** `src/routes/instances.rs:205` — `Path::join(&form.name)` with no validation → `std::fs::create_dir_all()`
62
63### P8-001: MOTD Docker Environment Injection (HIGH)
64
65The MOTD field is passed directly into Docker container environment variables without newline escaping. Docker treats `\n` in env values as variable delimiters, allowing injection of arbitrary env vars.
66
67**Code path:** `src/docker.rs:128` — `format!("MOTD={}", instance.motd)` → Docker container env → injected vars override security settings like `EULA`, `ONLINE_MODE`.
68
69### P8-002: RCON Command Injection (HIGH)
70
71The `player_action!` macro constructs RCON commands via `format!($command, form.username)` with no sanitization. Spaces, semicolons, and special characters in usernames can inject arbitrary RCON commands.
72
73**Code path:** `src/routes/players.rs:97` — `format!("op {}", form.username)` → `client.send_command_async(cmd)` → Minecraft server console.
74
75### P8-006: Missing CSRF Protection (HIGH)
76
77All 31 POST endpoints lack CSRF tokens. The session cookie uses `SameSite=Lax` which allows cross-site top-level POST navigations. Combined with other findings (path traversal, RCON injection, instance deletion), CSRF enables fully weaponized cross-site attacks.
78
79### P8-009: RCON Password Exposure (HIGH)
80
81The `GET /instances/{id}/edit` endpoint returns `rcon_password` for any instance to any authenticated user. No authorization check beyond `require_auth`.
82
83**Code path:** `src/routes/instances.rs:313` — `rcon_password: row.get(15)?` — rendered in `EditInstanceTemplate`.
84
85### P8-004: Level-Seed Properties Injection (HIGH)
86
87The `seed` field in world regeneration is written to `server.properties` without escaping. Newlines and special characters inject arbitrary Java `.properties` entries.
88
89**Code path:** `src/routes/instances.rs:397` — `format!("level-seed={seed}")` → `std::fs::write(&props_path)`.
90
91---
92
93## Cross-Model Attack Chains
94
95### Chain 1: CSRF + Path Traversal for Full System Compromise
961. CSRF (P8-006) tricks authenticated admin to create instance with traversed name
972. Path traversal (P8-003) enables writing arbitrary files outside data directory
983. Combined impact: full file system write without user interaction
99
100### Chain 2: RCON Password Exposure + Command Injection
1011. Any authenticated user views RCON password (P8-009)
1022. Same user exploits RCON command injection (P8-002) for server compromise
1033. Combined impact: horizontal privilege escalation to Minecraft server operator
104
105### Chain 3: Session Leak + No TTL
1061. Session cookie leaked through any channel (logs, DB backup, etc.)
1072. No server-side TTL (P8-005) means session never expires
1083. POST /logout (P8-007) allows anonymous invalidation of leaked session
1094. Combined impact: indefinite session hijacking window
110
111---
112
113## Coverage Summary
114
115| Entry Point | Finding Coverage | Notes |
116|------------|-----------------|-------|
117| POST /login | P8-006 (CSRF) | No rate limiting finding also relevant |
118| POST /logout | P8-006 (CSRF), P8-007 (Missing Auth) | Not behind require_auth |
119| GET /static/* | None | Safe — static files only |
120| POST /instances/new | P8-003 (Path Traversal), P8-006 (CSRF) | CRITICAL chain |
121| POST /instances/{id}/edit | P8-001 (MOTD injection), P8-006 (CSRF) | Docker env injection path |
122| POST /instances/{id}/delete | P8-003 (Path Traversal), P8-006 (CSRF) | Directory deletion |
123| POST /instances/{id}/activate | P8-001 (MOTD injection chain) | Docker container start |
124| POST /instances/{id}/world/regenerate | P8-004 (Properties Injection), P8-006 (CSRF) | server.properties injection |
125| GET /instances/{id}/edit | P8-009 (RCON password exposure) | Sensitive data leak |
126| POST /players/*/add, /players/*/remove | P8-002 (RCON command injection), P8-006 (CSRF) | 6 endpoints |
127| POST /instances/{id}/mods | P8-003 (Path traversal via mod name), P8-010 (DoS), P8-006 (CSRF) | File write + DoS |
128| POST /instances/{id}/mods/{mid}/delete | P8-003 (Path traversal), P8-006 (CSRF) | File delete |
129| POST /instances/{id}/mods/rescan | P8-010 (DoS), P8-006 (CSRF) | File read + validation |
130| GET /instances/{id}/world/export | P8-003 (Path traversal read) | File read |
131| POST /instances/{id}/world/import | P8-003 (Path traversal write), P8-006 (CSRF) | File write (ZIP) |
132| GET /backups/{id}/download | P8-003 (Path traversal read) | File read |
133| POST /instances/{id}/backups/create | P8-006 (CSRF) | File write |
134| GET /accounts, POST /accounts/* | P8-006 (CSRF) | Superuser operations |
135| POST /change-password | P8-006 (CSRF) | Password change via CSRF |
136| All authenticated routes | P8-005 (No session TTL) | Persistent session hijacking |
137| Session management | P8-005, P8-007 | No TTL, logout without auth |
138
139---
140
141## Missing Coverage / Gaps
142
1431. **No rate limiting** — `POST /login` has no brute-force protection (existing p4-005, not re-drafted here)
1442. **No CSRF on any POST** — universal issue (P8-006)
1453. **Flat authorization model** — any authenticated user can access any instance (by-design for single-user/small-team)
1464. **No audit logging** — no record of state-changing operations (existing p4-017)
1475. **No encryption at rest** — SQLite database unencrypted on disk (existing p4-021)
1486. **No backup restore functionality** — `restore_backup` is a stub (existing p4-024)
1497. **Cookie without Secure flag** — missing when behind HTTPS proxy (existing p4-009)
150
151---
152
153## Recommendations (Priority Order)
154
1551. **CRITICAL: Sanitize instance name** — validate against `^[a-zA-Z0-9_-]+$` before filesystem use
1562. **HIGH: Strip newlines from Docker env vars** — sanitize MOTD and all user-controlled fields before Docker env injection
1573. **HIGH: Validate RCON usernames** — restrict to `[a-zA-Z0-9_]` (3-16 chars) before command construction
1584. **HIGH: Add CSRF protection** — SameSite=Strict cookie + CSRF tokens or origin validation
1595. **HIGH: Remove RCON password from edit form** — expose only to superusers
1606. **HIGH: Escape seed value in server.properties** — reject newlines and control characters
1617. **MEDIUM: Add server-side session TTL** — filter sessions by `created_at > datetime('now', '-1 day')`
1628. **MEDIUM: Move logout behind require_auth** — defense-in-depth
1639. **MEDIUM: Add timeout to mod validation** — prevent DoS via crafted JARs
16410. **LOW: Use OsRng for RCON password** — cryptographic PRNG for secrets