summaryrefslogtreecommitdiffstats
path: root/piolium/attack-surface/variant-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/variant-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/variant-summary.md')
-rw-r--r--piolium/attack-surface/variant-summary.md130
1 files changed, 130 insertions, 0 deletions
diff --git a/piolium/attack-surface/variant-summary.md b/piolium/attack-surface/variant-summary.md
new file mode 100644
index 0000000..0503ac1
--- /dev/null
+++ b/piolium/attack-surface/variant-summary.md
@@ -0,0 +1,130 @@
1# Variant Analysis Summary — Phase 12
2
3**Generated:** 2025-07-29
4**Phase:** P12 (Variant Search)
5**Status:** COMPLETE
6
7---
8
9## Methodology
10
11For each of the 17 surviving Phase 10 findings, the entire codebase was searched for:
121. Same root cause pattern (unsanitized input → dangerous sink)
132. Same flow shape (source type → transformation → sink type)
143. Same attacker control level (authenticated/unauthenticated, direct/indirect)
154. Missing protections (no input validation, no escaping, no authorization check)
16
17---
18
19## Results
20
21| Finding | Root Cause | Variants Found | New Drafts |
22|---------|-----------|----------------|------------|
23| p10-001 — Instance Name Path Traversal | Path::join() with unsanitized user string | **4 new** (world import, regenerate, player mgmt, backup create) | p12-001 through p12-004 |
24| p10-002 — RCON Command Injection | format!() with unsanitized username in RCON command | 0 | — |
25| p10-003 — Level-Seed Properties Injection | format!() with unsanitized seed in Java .properties file | 0 | — |
26| p10-004 — RCON Password Exposed | Sensitive field in template rendering | 0 | — |
27| p10-005 — Missing CSRF Protection | Architectural (no CSRF tokens anywhere) | 0 (all 31 endpoints) | — |
28| p10-006 — No Server-Side Session TTL | Single query point | 0 | — |
29| p10-007 — No Rate Limiting Login | Architectural (no middleware) | 0 | — |
30| p10-008 — Logout No Auth Middleware | Single route registration | 0 | — |
31| p10-009 — Cookie No Secure Flag | Single cookie setting | 0 | — |
32| p10-010 — Flat Authorization Model | Architectural (no per-resource authz) | 0 | — |
33| p10-011 — Background Task Stale is_active | Background task closes over stale DB filter | 0 (all 5 updates in same task) | — |
34| p10-012 — Delete-Create Name Reuse Race | TOCTOU race window | 0 | — |
35| p10-013 — No Idempotency | Architectural (no idempotency keys) | 0 | — |
36| p10-014 — Mod Write Before DB Orphan | Ordering bug (file before DB insert) | 0 | — |
37| p10-015 — Backup Orphan Race | Ordering bug (ZIP before DB insert) | 0 | — |
38| p10-016 — Mod Validation Denial of Service | No timeout on CPU-bound parsing | 0 | — |
39| p10-017 — No Encryption at Rest | Architectural (no SQLite encryption) | 0 | — |
40
41---
42
43## Path Traversal Variant Details
44
45### Variant 1: World Import Path Traversal (p12-001)
46
47| Property | Value |
48|----------|-------|
49| Source | `instances.name` from DB (originally `form.name`) |
50| Sink | `std::fs::File::create()` at `instances.rs:704` |
51| Flow | POST `/instances/{id}/world/import` → load name from DB → `.join(&instance_name)` → `world_dir.join(zip_entry)` → `File::create()` |
52| Constraint | `/data/world/` suffix appended after traversed name |
53| Severity | HIGH |
54| New? | Yes — NOT in AP-001 registry previously |
55
56### Variant 2: Regenerate World Path Traversal (p12-002)
57
58| Property | Value |
59|----------|-------|
60| Source | `instances.name` from DB (originally `form.name`) |
61| Sink | `std::fs::remove_dir_all()` at `instances.rs:779` |
62| Flow | POST `/instances/{id}/world/regenerate` → load name from DB → `.join(&instance_name)` → `remove_dir_all(world_dir)` |
63| Constraint | `/data/world/` suffix appended after traversed name |
64| Severity | HIGH |
65| New? | Yes — NOT in AP-001 registry previously |
66
67### Variant 3: Player Management Path Traversal (p12-003)
68
69| Property | Value |
70|----------|-------|
71| Source | `instances.name` from active instance query |
72| Sink | `std::fs::read_to_string()` via `read_json_list()` at `players.rs:59-76` |
73| Flow | GET `/players` → `get_active_instance()` → `.join(&inst_name)` → `read_json_list(.../whitelist.json)` |
74| Constraint | Only files named `whitelist.json`, `ops.json`, `banned-players.json` are read |
75| Severity | MEDIUM (read-only, constrained to 3 filenames) |
76| New? | Yes — NOT in AP-001 registry previously |
77
78### Variant 4: Backup Create Path Traversal (p12-004)
79
80| Property | Value |
81|----------|-------|
82| Source | `instances.name` from DB (originally `form.name`) |
83| Sink | `std::fs::File::create()` at `backups.rs:114` |
84| Flow | POST `/instances/{id}/backups/create` → load name from DB → `filename = {name}_{timestamp}.zip` → `.join(&filename)` → `File::create()` |
85| Constraint | Timestamp suffix (`_YYYYMMDD_HHMMSS.zip`) attached to traversed name |
86| Severity | MEDIUM (cannot control exact filename) |
87| New? | Partially — backup filename path was listed in registry but created via Write (not Read) |
88
89---
90
91## No Variants Found — Rationale
92
93### AP-002 (RCON Injection)
94The `player_action!` macro generates 6 endpoints with unsanitized username → RCON command. All other RCON calls use hardcoded commands ("seed", "list"). No unsanitized input reaches any other `send_command_async()` call.
95
96### AP-003 (Properties Injection)
97Only `regenerate_world` writes to `server.properties` with user data. The seed is also passed to Docker as `LEVEL_SEED` env var (docker.rs:141), but Docker API injection was ruled FALSE POSITIVE in Phase 10. No other Java .properties files are written with user data.
98
99### AP-004/005 (CSRF, Session TTL)
100Architectural/design-level vulnerabilities. CSRF affects all 31 POST endpoints universally. Session TTL is a single missing WHERE clause in one query. No code-level variants.
101
102### Concurrency (p10-011 through p10-015)
103Each concurrency finding is a unique race condition or ordering bug in a specific code flow. No identical patterns exist elsewhere in the codebase. The `WHERE is_active = 1` filter appears in the dashboard (read-only) and player management (read-only via `get_active_instance`), but these are synchronous reads that always return current state — unlike the background task which closes over the `is_active` state captured at task creation time.
104
105### DoS (p10-016)
106The constant pool parser in `scan_single_class()` is the only CPU-bound parsing without a timeout. No other validation function lacks a timeout. `read_metadata()` opens a ZipArchive but its parsing is bounded by the ZIP entry count and file size.
107
108### Encryption (p10-017)
109Single architectural gap — no encryption layer on SQLite. No variants.
110
111---
112
113## Registry Updates
114
115The following new instances were added to AP-001 in the attack pattern registry:
116- `src/routes/instances.rs:672 — world import via instance_name (FILE WRITE)`
117- `src/routes/instances.rs:776 — regenerate world via instance_name (DIRECTORY DELETE)`
118- `src/routes/players.rs:53 — player management via inst_name (FILE READ)`
119- `src/routes/backups.rs:112 — backup create via filename (FILE WRITE)`
120
121---
122
123## Conclusion
124
125**Total new variants found:** 4
126**All from:** AP-001 (Path Traversal via Unsanitized Instance Name)
127**Total variant drafts written:** 4 (p12-001 through p12-004)
128**Findings with zero variants:** 13 (p10-002 through p10-017, except p10-001)
129
130The path traversal pattern (AP-001) is the most pervasive vulnerability class in the codebase, with now 12 confirmed instances across 4 route modules (instances.rs, mods.rs, backups.rs, players.rs).