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.
---
 .../attack-surface/public-routes-authz-matrix.md   | 92 ++++++++++++++++++++++
 1 file changed, 92 insertions(+)
 create mode 100644 piolium/attack-surface/public-routes-authz-matrix.md

(limited to 'piolium/attack-surface/public-routes-authz-matrix.md')

diff --git a/piolium/attack-surface/public-routes-authz-matrix.md b/piolium/attack-surface/public-routes-authz-matrix.md
new file mode 100644
index 0000000..d8f9389
--- /dev/null
+++ b/piolium/attack-surface/public-routes-authz-matrix.md
@@ -0,0 +1,92 @@
+# Authorization Matrix — Public Routes & Access Control
+
+**Coverage stats**: 37 endpoints discovered | 1 endpoint with no guard detected (missing-guard) | 24 endpoints taking object-id parameter
+**Coverage gaps**: None — all routes are statically registered in the axum Router. No reflection-based or dynamically-registered handlers.
+
+## Legend
+
+| Column | Description |
+|--------|-------------|
+| Layer-1 Guard | Declarative decorators/annotations (none in this Rust axum app) |
+| In-body Authz | Handler-level checks (superuser guard, ownership filters, etc.) |
+| Router/Middleware Guard | `require_auth` middleware applied at the router-group level |
+| Hidden Control Channels | Request-derived values that influence auth, routing, or identity |
+| Object-ID Param | Path parameter that identifies a resource (potential IDOR vector) |
+| Ownership Check? | Does the handler filter queries by the acting user's identity? |
+| Tenant Filter? | Does the handler filter by tenant/org scope? |
+| Expected Scope | `public` / `authenticated` / `role:superuser` / `unknown` |
+
+## Matrix
+
+| # | Method | Path | Handler (file:line) | Layer-1 Guard | In-body Authz | Router/Middleware Guard | Hidden Control Channels | Object-ID Param | Ownership Check? | Tenant Filter? | Expected Scope | Notes |
+|---|--------|------|---------------------|---------------|---------------|-------------------------|-------------------------|------------------|------------------|----------------|----------------|-------|
+| 1 | GET | `/login` | `login_page` (src/routes/mod.rs:53) | none | none | none | none | none | N/A | N/A | **public** | Login page — no session required |
+| 2 | POST | `/login` | `authenticate` (src/auth.rs:34) | none | none | none | Cookie set on response, no proxy-derived identity | none | N/A | N/A | **public** | Auth entry point — credentials → session |
+| 3 | POST | `/logout` | `logout` (src/auth.rs:130) | none | none | **missing guard** — not behind `require_auth` | Cookie header parsed for session ID | none | N/A | N/A | **public (anomaly: expected authenticated)** | ⚠️ Missing guard — see p4-008 |
+| 4 | GET | `/static/*` | `ServeDir` (tower-http, src/routes/mod.rs:28) | none | none | none | File path after `/static/` | file path | N/A | N/A | **public** | Static files (CSS, JS only) |
+| 5 | GET | `/` | `dashboard` (src/routes/dashboard.rs:19) | none | none | `require_auth` | Cookie → CurrentUser via `FromRequestParts` | none | N/A | N/A | **authenticated** | Dashboard — requires valid session |
+| 6 | GET | `/instances` | `list_instances` (src/routes/instances.rs:44) | none | none | `require_auth` | Cookie → CurrentUser | none | N/A (all instances shared) | N/A (single-tenant) | **authenticated** | Lists all instances — no user scoping |
+| 7 | GET | `/instances/new` | `create_form` (src/routes/instances.rs:70) | none | none | `require_auth` | Cookie → CurrentUser | none | N/A | N/A | **authenticated** | Instance creation form |
+| 8 | POST | `/instances/new` | `create_instance` (src/routes/instances.rs:87) | none | none | `require_auth` | Cookie → CurrentUser | none | N/A | N/A | **authenticated** | Creates instance (name used as dir name) |
+| 9 | GET | `/instances/{id}/edit` | `edit_form` (src/routes/instances.rs:129) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** — queries by id without user filter | N/A | **authenticated** | Edit form — any auth user can edit any instance |
+| 10 | POST | `/instances/{id}/edit` | `edit_instance` (src/routes/instances.rs:176) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Updates any instance by id |
+| 11 | POST | `/instances/{id}/delete` | `delete_instance` (src/routes/instances.rs:198) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Deletes any instance — destructive |
+| 12 | POST | `/instances/{id}/activate` | `activate_instance` (src/routes/instances.rs:217) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Activates instance → Docker container start |
+| 13 | POST | `/instances/deactivate` | `deactivate_instance` (src/routes/instances.rs:278) | none | none | `require_auth` | Cookie → CurrentUser | none | N/A | N/A | **authenticated** | Deactivates active instance |
+| 14 | GET | `/instances/{id}/world/export` | `export_world` (src/routes/instances.rs:288) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Exports world ZIP — file read |
+| 15 | POST | `/instances/{id}/world/import` | `import_world` (src/routes/instances.rs:328) | none | none | `require_auth` | Cookie → CurrentUser; Multipart Content-Type/boundary | `id` (i64, path) | **No** | N/A | **authenticated** | Imports world ZIP — file write |
+| 16 | POST | `/instances/{id}/world/regenerate` | `regenerate_world` (src/routes/instances.rs:360) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Regenerates world — file write |
+| 17 | GET | `/players` | `manage_players` (src/routes/players.rs:28) | none | none | `require_auth` | Cookie → CurrentUser | none | N/A | N/A | **authenticated** | Player management page |
+| 18 | POST | `/players/whitelist/add` | `add_whitelist` (src/routes/players.rs:89, macro) | none | none | `require_auth` | Cookie → CurrentUser; RCON password from DB | none (username in form) | N/A | N/A | **authenticated** | RCON command — any auth user can manage whitelist |
+| 19 | POST | `/players/whitelist/remove` | `remove_whitelist` (src/routes/players.rs:89, macro) | none | none | `require_auth` | Same as #18 | none | N/A | N/A | **authenticated** | RCON command |
+| 20 | POST | `/players/op/add` | `add_op` (src/routes/players.rs:89, macro) | none | none | `require_auth` | Same as #18 | none | N/A | N/A | **authenticated** | RCON command — any auth user can op players |
+| 21 | POST | `/players/op/remove` | `remove_op` (src/routes/players.rs:89, macro) | none | none | `require_auth` | Same as #18 | none | N/A | N/A | **authenticated** | RCON command |
+| 22 | POST | `/players/ban/add` | `add_ban` (src/routes/players.rs:89, macro) | none | none | `require_auth` | Same as #18 | none | N/A | N/A | **authenticated** | RCON command — any auth user can ban players |
+| 23 | POST | `/players/ban/remove` | `remove_ban` (src/routes/players.rs:89, macro) | none | none | `require_auth` | Same as #18 | none | N/A | N/A | **authenticated** | RCON command |
+| 24 | GET | `/instances/{id}/backups` | `list_backups` (src/routes/backups.rs:30) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Lists backups for any instance |
+| 25 | POST | `/instances/{id}/backups/create` | `create_backup` (src/routes/backups.rs:54) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Creates backup — file write |
+| 26 | POST | `/backups/{id}/restore` | `restore_backup` (src/routes/backups.rs:88) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | STUB — no-op redirect |
+| 27 | GET | `/backups/{id}/download` | `download_backup` (src/routes/backups.rs:93) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Downloads backup ZIP — file read |
+| 28 | GET | `/instances/{id}/mods` | `list_mods` (src/routes/mods.rs:42) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Lists mods for any instance |
+| 29 | POST | `/instances/{id}/mods` | `upload_mod` (src/routes/mods.rs:86) | none | none | `require_auth` | Cookie → CurrentUser; Content-Type boundary for multer | `id` (i64, path) | **No** | N/A | **authenticated** | Uploads mod — file write, JAR parsing |
+| 30 | POST | `/instances/{id}/mods/{mod_id}/delete` | `delete_mod` (src/routes/mods.rs:188) | none | none | `require_auth` | Cookie → CurrentUser | `instance_id`, `mod_id` (path) | **No** | N/A | **authenticated** | Deletes mod — file delete |
+| 31 | POST | `/instances/{id}/mods/rescan` | `rescan_mods` (src/routes/mods.rs:163) | none | none | `require_auth` | Cookie → CurrentUser | `id` (i64, path) | **No** | N/A | **authenticated** | Rescans mods directory — file read + DB insert |
+| 32 | GET | `/accounts` | `manage_accounts` (src/routes/accounts.rs:60) | none | `if !account.is_superuser` (line 107) | `require_auth` | Cookie → CurrentUser | none | N/A | N/A | **role:superuser** | Account list — superuser inline check |
+| 33 | POST | `/accounts/create` | `create_account` (src/routes/accounts.rs:95) | none | `if !account.is_superuser` (line 145) | `require_auth` | Cookie → CurrentUser | none (form fields) | N/A | N/A | **role:superuser** | Create account — superuser inline check |
+| 34 | POST | `/accounts/delete` | `delete_account` (src/routes/accounts.rs:111) | none | `if !account.is_superuser` (line 167) | `require_auth` | Cookie → CurrentUser | `form.id` (body) | N/A | N/A | **role:superuser** | Delete account — superuser inline check |
+| 35 | POST | `/accounts/set-password` | `set_password` (src/routes/accounts.rs:142) | none | `if !account.is_superuser` (line 204) | `require_auth` | Cookie → CurrentUser | `form.id` (body) | N/A | N/A | **role:superuser** | Set password — superuser inline check |
+| 36 | GET | `/change-password` | `change_password_page` (src/routes/mod.rs:58) | none | none | `require_auth` | Cookie → CurrentUser (`FromRequestParts`) | none | N/A | N/A | **authenticated** | Password change page — requires auth |
+| 37 | POST | `/change-password` | `accounts::change_password` (src/routes/accounts.rs:163) | none | none | `require_auth` | Cookie → CurrentUser | none | Self (uses `account.id` from auth) | N/A | **authenticated** | Change own password — verifies current password |
+| 38 | POST | `/accounts/change-own-password` | `change_own_password_from_accounts` (src/routes/accounts.rs:163) | none | none | `require_auth` | Cookie → CurrentUser | none | Self (uses `account.id` from auth) | N/A | **authenticated** | Duplicate of #37, same handler |
+
+## Anomaly Summary
+
+| # | Anomaly | Routes Affected | Severity | Existing Draft |
+|---|---------|-----------------|----------|----------------|
+| A1 | **Missing guard** — POST /logout registered on outer router, not behind `require_auth` middleware | #3 | MEDIUM | p4-008 |
+| A2 | **Fragile superuser pattern** — superuser check done inline in handler body via `if !account.is_superuser` instead of middleware/extractor | #32, #33, #34, #35 | LOW (pattern concern) | p4-025 |
+| A3 | **No ownership/tenant scoping** — all authenticated users have equal access to ALL instances, mods, backups, player management | #6–#31 | INFO (by design: single-user/small-team) | — |
+| A4 | **No CSRF protection** — all POST endpoints lack CSRF tokens | #2, #3, #8, #10, #11, #12, #13, #15, #16, #18–#23, #25, #26, #29, #30, #31, #33–#35, #37, #38 | HIGH | p4-003 |
+| A5 | **Hidden control channel** — `Content-Type` boundary parsed for multer multipart (attacker-controlled boundary string) | #29 | LOW | — |
+| A6 | **Hidden control channel** — Cookie header parsing relies on raw string splitting (no validation of session ID format) | All protected routes | LOW | p4-019 |
+
+## Hidden Control Channels Map
+
+| Channel | Where Used | What It Controls | Can It Bypass Auth? |
+|---------|-----------|-----------------|---------------------|
+| `Cookie: mcui_session` | `require_auth` (src/auth.rs:55), `logout` (src/auth.rs:130) | Identity (session lookup → Account) | No — session must exist in DB |
+| `Content-Type: boundary=` | `upload_mod` (src/routes/mods.rs:86) | Multipart parsing boundary | No — only affects file parsing |
+| Path parameters `{id}` | All `/instances/{id}/*`, `/backups/{id}/*` routes | Resource selection | No — but no ownership check on selected resource |
+| Form fields | All POST handlers | Operation parameters | No — parametrized queries prevent injection |
+| X-Forwarded-* Headers | **Not read anywhere** | N/A | N/A (safe — not trusted) |
+| X-HTTP-Method-Override | **Not supported** | N/A | N/A (safe) |
+| X-Admin / X-Internal | **Not supported** | N/A | N/A (safe) |
+
+## Authz Architecture Summary
+
+- **Authentication**: Cookie-based session (UUIDv4) via `require_auth` middleware (src/auth.rs:55)
+- **Authorization model**: Flat — all authenticated users are equal. No per-resource ownership, no roles beyond `is_superuser`.
+- **Superuser gating**: Inline `if !account.is_superuser` checks in handler bodies (src/routes/accounts.rs)
+- **Static file serving**: Completely public
+- **Logout**: Public (missing guard — only by design because session IDs are unpredictable UUIDs)
+- **Session management**: 24h max-age cookie, NO server-side TTL check, NO Secure flag
-- 
cgit v1.2.3