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. --- piolium/attack-surface/advisory-summary.md | 395 ++++++++++++++ piolium/attack-surface/architecture-entrypoints.md | 137 +++++ piolium/attack-surface/candidates-summary.md | 55 ++ piolium/attack-surface/candidates.jsonl | 26 + piolium/attack-surface/cross-service-edges.json | 1 + piolium/attack-surface/deep-cleanup-summary.json | 34 ++ piolium/attack-surface/deep-probe-summary.md | 164 ++++++ piolium/attack-surface/knowledge-base-report.md | 597 +++++++++++++++++++++ piolium/attack-surface/lite-recon.md | 37 ++ .../manual-attack-surface-inventory.md | 311 +++++++++++ piolium/attack-surface/patch-bypass-summary.md | 305 +++++++++++ .../attack-surface/public-routes-authz-matrix.md | 92 ++++ piolium/attack-surface/sbom.json | 282 ++++++++++ .../source-sink-flows-all-severities.md | 143 +++++ piolium/attack-surface/spec-gap-summary.md | 164 ++++++ .../attack-surface/state-concurrency-summary.md | 46 ++ piolium/attack-surface/unauthenticated-surface.md | 110 ++++ piolium/attack-surface/variant-summary.md | 130 +++++ piolium/audit-state.json | 128 +++++ piolium/final-audit-report.md | 407 ++++++++++++++ 20 files changed, 3564 insertions(+) create mode 100644 piolium/attack-surface/advisory-summary.md create mode 100644 piolium/attack-surface/architecture-entrypoints.md create mode 100644 piolium/attack-surface/candidates-summary.md create mode 100644 piolium/attack-surface/candidates.jsonl create mode 100644 piolium/attack-surface/cross-service-edges.json create mode 100644 piolium/attack-surface/deep-cleanup-summary.json create mode 100644 piolium/attack-surface/deep-probe-summary.md create mode 100644 piolium/attack-surface/knowledge-base-report.md create mode 100644 piolium/attack-surface/lite-recon.md create mode 100644 piolium/attack-surface/manual-attack-surface-inventory.md create mode 100644 piolium/attack-surface/patch-bypass-summary.md create mode 100644 piolium/attack-surface/public-routes-authz-matrix.md create mode 100644 piolium/attack-surface/sbom.json create mode 100644 piolium/attack-surface/source-sink-flows-all-severities.md create mode 100644 piolium/attack-surface/spec-gap-summary.md create mode 100644 piolium/attack-surface/state-concurrency-summary.md create mode 100644 piolium/attack-surface/unauthenticated-surface.md create mode 100644 piolium/attack-surface/variant-summary.md create mode 100644 piolium/audit-state.json create mode 100644 piolium/final-audit-report.md (limited to 'piolium') diff --git a/piolium/attack-surface/advisory-summary.md b/piolium/attack-surface/advisory-summary.md new file mode 100644 index 0000000..eaea57a --- /dev/null +++ b/piolium/attack-surface/advisory-summary.md @@ -0,0 +1,395 @@ +# Advisory Summary: minecraft-ui + +**Generated**: 2025-07-29T01:15:00Z +**Target**: `salmon/minecraft-ui` (self-hosted git at `git.2137697.xyz/salmon/minecraft-ui.git`) +**Analysis scope**: Stage 01 — Intelligence & Dependency Risk + +--- + +## Repository Identity + +| Field | Value | +|---|---| +| OWNER | `salmon` | +| REPO | `minecraft-ui` | +| OWNER_REPO | `salmon/minecraft-ui` | +| Resolution source | `git remote` (`https://git.2137697.xyz/salmon/minecraft-ui.git`) | +| Git history available | Yes (6 commits) | +| GitHub API available | No — self-hosted git server, not github.com | +| Primary ecosystem | Rust (Cargo) — edition 2024 | +| Total lines of code | ~3,213 (Rust source), 6 templates (Askama), 1 CSS, 1 JS (htmx) | + +## Coverage Gaps + +- **Source 1 (git log CVE search)**: No CVE/GHSA references found in repo (0 results) — new project, no changelog, no SECURITY.md +- **Source 2 (GitHub Security Advisories)**: **Skipped** — `gh api` requires github.com; repo lives on `git.2137697.xyz` (self-hosted Gitea/Forgejo) +- **Source 5 (supplementary websearch)**: Not performed — Sources 1, 3, 4 cover all dependency advisories; no project-specific CVEs expected for a new internal tool + +--- + +## Advisory Inventory + +### Direct Dependencies — Vulnerability Status + +All 22 direct dependencies are at versions past all known advisories. No unfixed public vulnerabilities in the direct dependency tree. + +| # | Package | Version | Known Advisories | Affected Range | Current Status | +|---|---------|---------|-----------------|---------------|----------------| +| 1 | `axum` | 0.8.9 | 0 | — | ✅ Clean | +| 2 | `multer` | 3.1.0 | 0 | — | ✅ Clean | +| 3 | `tokio` | 1.52.3 | 5 (historic) | All < 1.38.2 (oldest fix) | ✅ Fixed well above | +| 4 | `rusqlite` | 0.40.1 | 15 (historic) | < 0.25.4 (data races), < 0.26.2 (UAF) | ✅ Fixed well above | +| 5 | `bollard` | 0.17.1 | 2 | Checked — no active vulns | ✅ Clean | +| 6 | `argon2` | 0.5.3 | 0 | — | ✅ Clean | +| 7 | `zip` | 8.6.0 | **1 (CVE-2025-29787)** | 1.3.0 ≤ v < 2.3.0 | ✅ Fixed (v8.6.0 >> fix) | +| 8 | `reqwest` | 0.13.4 | 0 | — | ✅ Clean | +| 9 | `cookie` | 0.18.1 | 2 (historic) | < 0.7.6 | ✅ Fixed well above | +| 10 | `serde` | 1.0.228 | 0 | — | ✅ Clean | +| 11 | `askama` | 0.16.0 | 0 | — | ✅ Clean | +| 12 | `tower-http` | 0.7.0 (+ 0.6.11 transitive) | 2 (historic dir traversal) | < 0.1.3 / < 0.2.1 | ✅ Fixed well above | +| 13 | `axum-extra` | 0.12.6 | 0 | — | ✅ Clean | +| 14 | `tower` | 0.5.3 | 0 | — | ✅ Clean | +| 15 | `chrono` | 0.4.45 | 0 | — | ✅ Clean | +| 16 | `uuid` | 1.23.4 | 0 | — | ✅ Clean | +| 17 | `tempfile` | 3.27.0 | 3 | All historic/pre-2022 | ✅ Clean | +| 18 | `rand` | 0.8.7 | 1 (RUSTSEC-2026-0097) | 0.7.0 ≤ v < 0.8.6 | ✅ Fixed (0.8.7) | +| 19 | `tracing` | 0.1.44 | 1 (RUSTSEC-2023-0078 UAF) | 0.1.38 ≤ v < 0.1.40 | ✅ Fixed (0.1.44) | +| 20 | `serde_json` | 1.0.150 | 0 | — | ✅ Clean | +| 21 | `toml` | 1.1.2 | 0 | — | ✅ Clean | +| 22 | `futures-util` | 0.3.32 | 0 | — | ✅ Clean | + +### Notable Transitive Dependencies + +| Package | Version | Known Advisories | Status | +|---------|---------|-----------------|--------| +| `hyper` | 1.10.1 | 0 | ✅ Clean | +| `h2` | 0.4.15 | 3 (all fixed in 0.4.4+) | ✅ Clean (0.4.15 > 0.4.4) | +| `mio` | 1.2.1 | 0 | ✅ Clean | +| `aes-gcm` | 0.10.3 | 0 | ✅ Clean | + +### Detailed Advisory Records + +#### 1. CVE-2025-29787 — zip crate path traversal (HIGH) + +| Field | Value | +|---|---| +| **ID** | `GHSA-94vh-gphv-8pm8` / `CVE-2025-29787` | +| **Published** | 2025-03-17 | +| **Severity** | HIGH (CVSS:4.0 8.1) | +| **Summary** | `zip` incorrectly canonicalizes paths during archive extraction, leading to arbitrary file write via path traversal | +| **CWEs** | CWE-180 (Incorrect Behavior Order), CWE-22 (Path Traversal), CWE-61 (UNIX Symbolic Link) | +| **Affected** | zip >= 1.3.0, < 2.3.0 | +| **Fixed in** | zip 2.3.0 | +| **Current** | 8.6.0 — **Not affected** | +| **Relevance** | The project uses `zip::ZipArchive` in `mods.rs` (mod extraction from ZIP), `routes/instances.rs` (world import), and `routes/backups.rs` (backup creation). All are using version 8.6.0, past the fix. | + +#### 2. RUSTSEC-2025-0023 — tokio broadcast channel soundness + +| Field | Value | +|---|---| +| **ID** | `GHSA-rr8g-9fpq-6wmg` | +| **Published** | 2025-04-07 | +| **Severity** | Informational (unsoundness) | +| **Summary** | `tokio::sync::broadcast` channel calls `clone` in parallel but does not require `Sync` | +| **Affected** | tokio >= 0.2.5 (various ranges) | +| **Fixed in** | 1.38.2 / 1.43.1 / 1.44.2 | +| **Current** | 1.52.3 — **Not affected** | + +#### 3. RUSTSEC-2024-0332 — h2 CONTINUATION flood (HIGH) + +| Field | Value | +|---|---| +| **ID** | `GHSA-q6cp-qfwq-4gcv` / `RUSTSEC-2024-0332` | +| **Published** | 2024-04-03 | +| **Severity** | HIGH | +| **Summary** | h2 servers vulnerable to degradation of service with CONTINUATION flood frames | +| **Affected** | h2 < 0.3.26 / < 0.4.4 | +| **Fixed in** | 0.4.4 | +| **Current** | 0.4.15 — **Not affected** | + +#### 4. RUSTSEC-2023-0078 — tracing use-after-free + +| Field | Value | +|---|---| +| **ID** | `GHSA-8f24-6m29-wm2r` | +| **Published** | 2024-01-17 | +| **Severity** | Informational | +| **Summary** | Use-after-free in tracing subscriber when `MakeVisitor` is implemented on a type that implements `Visit` without `Clone` | +| **Affected** | tracing >= 0.1.38, < 0.1.40 | +| **Fixed in** | 0.1.40 | +| **Current** | 0.1.44 — **Not affected** | + +--- + +## Vulnerability Pattern Analysis + +### 2a. Component Vulnerability Heatmap + +The project is a **brand-new codebase** with zero first-party CVEs. The heatmap below is derived from dependency advisory history and architectural analysis: + +| Component | Advisory Count (historic) | Risk Factors | Dominant Bug Types | +|-----------|-------------------------|--------------|--------------------| +| **`rusqlite`** | 15 (all historic) | Data races (CVE-2020-35866..73), Use-After-Free (CVE-2021-45713..19) | Race conditions, UAF | +| **`tokio`** | 5 (all historic) | Race conditions, unsoundness | Concurrency bugs | +| **`h2`** (transitive) | 3 (all fixed by 0.4.4) | CONTINUATION flood, DoS | Resource exhaustion | +| **`zip`** | 1 (CVE-2025-29787, fixed in 2.3.0) | Path traversal, symlink | Path traversal | +| **`cookie`** | 2 (both historic, < 0.7.6) | Max-Age DoS, input validation | Input validation | +| **`tempfile`** | 3 (all historic) | Race conditions, permissions | TOCTOU | +| **`rand`** | 1 (RUSTSEC-2026-0097, fixed in 0.8.6) | Unsound with custom logger | Unsoundness | + +**🔥 High-heat components requiring Phase 3/5 attention:** +1. **`mods.rs` (480 lines)** — Custom JAR/ZIP parser, class-file constant pool scanning, ZIP extraction. Highest complexity, most attack surface. +2. **`routes/instances.rs` (782 lines)** — World import/export (ZIP extraction), instance lifecycle, Docker orchestration. Largest single file. +3. **`auth.rs` (199 lines)** — Session management, password hashing, cookie handling. Custom session cookie parsing (no library). +4. **`docker.rs` (209 lines)** — Docker API calls with bollard, container creation with user-controlled env vars. +5. **`rcon.rs` (127 lines)** — Raw TCP protocol implementation, password handling. + +### 2b. Bug Type Recurrence + +| Bug Class | CWEs | Historic Count | Examples | Recurring? | +|-----------|------|---------------|----------|------------| +| **Race condition / TOCTOU** | CWE-362 | 8+ | rusqlite data races, tokio races, tempfile | ⚠️ **Recurring** | +| **Path traversal** | CWE-22 | 2 | zip (CVE-2025-29787), tower-http (RUSTSEC-2022-0043) | ⚠️ **Recurring** | +| **Resource exhaustion / DoS** | CWE-400, CWE-770 | 3 | h2 CONTINUATION flood, cookie Max-Age, h2 general | ⚠️ **Recurring** | +| **Use-after-free** | CWE-416 | 8 | rusqlite UAF, tracing UAF | ⚠️ **Recurring** | +| **Improper input validation** | CWE-20 | 2 | cookie, zip | — | +| **Unsoundness** | — | 2 | tokio broadcast, rand | — | + +**Key insight**: Path traversal recurs across the dependency tree (zip and tower-http). The project's custom ZIP extraction code (`routes/instances.rs:import_world`) should be audited for the same pattern. + +### 2c. Attack Surface Trends + +| Input Vector | Where Exploited | Risk Level | +|-------------|----------------|------------| +| **Uploaded ZIP/JAR files** | `routes/mods.rs`, `routes/instances.rs` (world import) | 🔴 **Critical** | +| **HTTP requests** (form data, multipart) | All routes via axum | 🟡 High | +| **Docker container env vars** | `docker.rs:recreate_container` — user-controlled (version, MOTD, etc.) | 🟡 High | +| **RCON network protocol** | `rcon.rs` — custom TCP implementation | 🟡 High | +| **SQL queries via rusqlite** | All DB operations (uses params! macro — safe) | 🟢 Low | +| **Cookie header parsing** | `auth.rs:require_auth` — manual cookie split | 🟡 Medium | +| **Template rendering** | Askama templates (compile-time, no injection) | 🟢 Low | +| **Config file** | `config.toml` parsed via toml crate | 🟢 Low | + +**Highest-risk attack surface**: The mod upload/validation pipeline (`routes/mods.rs` + `mods.rs`) accepts arbitrary JAR/ZIP uploads with a 500MB size limit and performs custom bytecode-level class scanning. + +### 2d. Patch Quality Signals + +No patch-bypass history exists for this project (new codebase, no patched vulnerabilities). However, several **architectural concerns** warrant attention: + +1. **`mods.rs` class-file scanner** — Custom constant pool parser (not using a library). Any bug in this parser could lead to mis-validation (accepting client-side mods that crash the server, or rejecting valid server mods). No fuzzing evidence exists. + +2. **`auth.rs` cookie session parsing** — Manual `split(';')` + `split_once('=')` parsing of the Cookie header. Equivalent to CWE-172 (Encoding Error). No library cookie parser used despite `cookie` crate being a dependency. + +3. **`docker.rs` env injection** — Instance name, MOTD, version, seed, etc. are directly interpolated into Docker container env vars. No sanitization of values that could contain shell-special characters or newlines (env var injection risk). + +4. **`rcon.rs` packet parsing** — Custom binary protocol parser with `i32::from_le_bytes` length prefix. Potential for integer underflow/overflow in length handling. + +--- + +## Architecture Inventory + +### Components + +| Component | File(s) | Purpose | Security Profile | +|-----------|---------|---------|-----------------| +| **HTTP Router** | `routes/mod.rs` | Axum router with auth middleware | Cookie-based session auth on protected routes | +| **Auth module** | `auth.rs` | Password hashing (Argon2), session creation, login/logout | Custom cookie parsing; sessions in SQLite | +| **Dashboard** | `routes/dashboard.rs` | Landing page showing active instance | Reads-only | +| **Instance Manager** | `routes/instances.rs` | CRUD for server instances, world import/export, activation | World ZIP import/extract; Docker lifecycle | +| **Mod Manager** | `routes/mods.rs`, `mods.rs` | JAR/ZIP upload, validation, class-file scanning | Largest attack surface: custom bytecode parser | +| **Player Manager** | `routes/players.rs` | RCON-based whitelist/OP/ban management | Sends commands via RCON to Minecraft server | +| **Backup Manager** | `routes/backups.rs` | Create/list/restore/download backups | ZIP creation, file read for download | +| **Account Manager** | `routes/accounts.rs` | Account CRUD, password change | Superuser-only; session invalidation on password change | +| **Docker Manager** | `docker.rs` | Container lifecycle via bollard API | Image pull, container create/start/stop with env vars | +| **RCON Client** | `rcon.rs` | Raw TCP RCON protocol implementation | Custom binary protocol parser | +| **Database** | `db.rs` | SQLite via rusqlite with connection pool (Arc) | WAL mode, foreign keys, migrations | +| **Config** | `config.rs` | TOML config loader | Static config, parsed at startup | + +### Transports + +| Transport | Direction | Protocol | Used By | +|-----------|-----------|----------|---------| +| **HTTP** | User ↔ WebUI | HTTP/1.1 (via axum/hyper) | All routes | +| **Docker socket** | WebUI ↔ Docker daemon | Unix socket (bollard) | `docker.rs` | +| **RCON** | WebUI ↔ Minecraft server | TCP, custom binary protocol on port 25575 | `rcon.rs` | +| **Mojang API** | WebUI → piston-meta.mojang.com | HTTPS (reqwest) | Fetching version manifest | +| **File system** | WebUI ↔ Disk | Standard I/O | World export/import, mod uploads, backups | + +### Trust Boundaries + +``` +┌─────────────────────────────────────────────────────────────────┐ +│ Internet / LAN │ +│ ┌────────────┐ HTTP :3000 ┌────────────────────────┐ │ +│ │ Browser │ ──────────────▶ │ WebUI (axum) │ │ +│ │ (HTMX/JS) │ │ minecraft-ui process │ │ +│ └────────────┘ │ - auth middleware │ │ +│ │ - cookie sessions │ │ +│ └──────┬─────────────────┘ │ +│ │ │ +│ ┌───────────────────────────────┼──────────────┐ │ +│ │ Docker daemon socket │ RCON TCP │ │ +│ ▼ ▼ │ │ +│ ┌─────────────────────┐ ┌───────────────────┐ │ │ +│ │ Docker Container │ │ Minecraft Server │ │ │ +│ │ itzg/minecraft-srv │ │ (inside Docker) │ │ │ +│ │ port 25565 │ │ port 25575 │ │ │ +│ │ port 25575 │ └───────────────────┘ │ │ +│ └─────────────────────┘ │ │ +│ │ │ +│ ┌────────────────────────────────────────────────────┐ │ │ +│ │ File System │ │ │ +│ │ - server_directory/instances/{name}/data/ │ │ │ +│ │ - server_directory/backups/ │ │ │ +│ │ - server_directory/mcui.db (SQLite) │ │ │ +│ └────────────────────────────────────────────────────┘ │ │ +└─────────────────────────────────────────────────────────────────┘ +``` + +### Execution Environments + +| Environment | Description | +|-------------|-------------| +| **Host OS** | Linux (presumed — Docker socket, Unix paths) | +| **Rust runtime** | Native binary via `tokio` async runtime | +| **Docker container** | `itzg/minecraft-server` with Java 8/17/21 depending on version | +| **SQLite** | Embedded database (WAL mode) | + +### Highest-Risk Flows (Phase 3 DFD priority) + +1. **Mod Upload → Validation → Disk Write** — External attacker-controlled JAR/ZIP → custom bytecode parser → file written to `mods/` directory +2. **World Import → ZIP Extraction → Disk Write** — Uploaded ZIP → `zip::ZipArchive::by_index` → `enclosed_name()` → file extraction to `world/` +3. **Instance Create → Docker Container Start** — User-controlled form fields → env var interpolation → Docker API call +4. **Backup Download → File Read → HTTP Response** — Backup ID → file read from disk → streamed to user (path traversal risk) +5. **Login → Cookie → Session** — Password verification → session creation → cookie parsing in auth middleware + +--- + +## Component Inventory + +### Runtime + +| Component | Version | Evidence | Security-relevant | +|-----------|---------|----------|-------------------| +| Rust | edition 2024 / stable | `Cargo.toml` | No | + +### Package Dependencies (direct) + +| Component | Version | Purpose | Security-relevant | +|-----------|---------|---------|-------------------| +| axum | 0.8.9 | HTTP framework | Yes — request parsing | +| multer | 3.1.0 | Multipart uploads | Yes — file upload handling | +| tokio | 1.52.3 | Async runtime | Yes — concurrency | +| rusqlite | 0.40.1 | SQLite database | Yes — SQL injection surface | +| bollard | 0.17.1 | Docker API client | Yes — container control | +| argon2 | 0.5.3 | Password hashing | Yes — auth credential handling | +| zip | 8.6.0 | ZIP archive handling | Yes — file extraction | +| reqwest | 0.13.4 | HTTP client | No — version manifest fetches | +| cookie | 0.18.1 | Cookie handling | Yes — session cookies | +| serde | 1.0.228 | Serialization | No | +| askama | 0.16.0 | Template engine | No — compile-time templates | +| tower-http | 0.7.0 | HTTP middleware (static files) | No | +| axum-extra | 0.12.6 | Cookie private jar | Yes — session cookie crypto | +| chrono | 0.4.45 | Date/time | No | +| uuid | 1.23.4 | Session ID generation | No | +| tempfile | 3.27.0 | Temporary files | No | +| rand | 0.8.7 | RNG for passwords | Yes — RCON password generation | +| tracing | 0.1.44 | Logging | No | +| toml | 1.1.2 | Config parsing | No | + +### Binary Dependencies + +| Component | Evidence | Purpose | Security-relevant | +|-----------|----------|---------|-------------------| +| Docker daemon | `bollard::Docker::connect_with_local_defaults()` | Container management | Yes — OS-level access | +| Java (JRE 8/17/21) | Docker `itzg/minecraft-server` images | Minecraft runtime | Yes — runs untrusted mod code | +| `itzg/minecraft-server` | Docker image | Minecraft server packaging | Yes — external image | + +### External Services + +| Service | Evidence | Purpose | Security-relevant | +|---------|----------|---------|-------------------| +| `piston-meta.mojang.com` | `reqwest::get(...)` in `fetch_minecraft_versions()` | Version manifest | No — read-only public API | + +### Datastores + +| Component | Evidence | Purpose | Security-relevant | +|-----------|----------|---------|-------------------| +| SQLite (rusqlite) | `db.rs` | Accounts, sessions, instances, mods | Yes — credential storage | +| Docker volume mounts | `docker.rs` | Minecraft server data | Yes — file system access | +| Server directory | `config.toml:server_directory` | All persistent data | Yes — file system root | + +### Coverage Gaps + +| Category | Status | Note | +|----------|--------|------| +| runtime | ✅ Covered | Rust edition 2024 | +| package | ✅ Covered | All 22 direct deps from Cargo.toml | +| framework | ✅ Covered | axum, askama | +| datastore | ✅ Covered | SQLite, Docker volumes | +| external-service | ✅ Covered | Mojang API | +| container-os | ⚠️ Partial | Dockerfile not in repo; uses `itzg/minecraft-server` images | +| build-ci | ℹ️ None detected | No CI config found in repo | +| binary | ✅ Covered | Docker, Java (via Docker) | +| vendored | ✅ Covered | `static/htmx.min.js` (vendored JS library) | + +--- + +## Dependency Intelligence + +### Security-Relevant Dependencies (derived view) + +| Dependency | Version | Risk Factors | Pattern Cross-Reference | +|-----------|---------|-------------|------------------------| +| **axum** | 0.8.9 | Input parsing, multipart handling | HTTP request surface (2c) | +| **multer** | 3.1.0 | Multipart file upload with 500MB limit | File upload attack surface (2c) | +| **rusqlite** | 0.40.1 | →historic: data races, UAF | Bug class: race conditions (2b) | +| **bollard** | 0.17.1 | Docker socket access, container control | Docker API attack surface (2c) | +| **argon2** | 0.5.3 | Password hashing — credential protection | Auth security (2c) | +| **zip** | 8.6.0 | →historic: path traversal (CVE-2025-29787) | Bug class: path traversal (2b) | +| **cookie** | 0.18.1 | →historic: DoS via Max-Age | Session handling (2c) | +| **axum-extra** | 0.12.6 | Cookie private jar (encryption) | Session security | +| **rand** | 0.8.7 | RCON password generation | Password strength | + +### Key Risk Observations + +1. **No regular dependency scanning configured** — No `cargo audit` or `cargo deny` in CI. Relying on manual version choice. +2. **`htmx.min.js` (vendored)** — Static file served via `tower-http::services::ServeDir`. Should verify integrity of vendored copy. +3. **`itzg/minecraft-server` Docker images** — External base images with Java runtimes. Image pull happens at runtime; no image hash pinning in code. +4. **RCON password generation** — Uses `rand::thread_rng()` (CSPRNG) with 16-char alphanumeric charset (~93 bits entropy). Adequate but no periodic rotation. + +--- + +## Audit Targeting Recommendations + +Based on pattern analysis: + +1. **Phase 3 DFD priorities**: + - **Mod upload/validation pipeline** (`routes/mods.rs` + `mods.rs`) — highest complexity, custom bytecode parser + - **World import/export** (`routes/instances.rs:import_world`, `export_world`) — ZIP extraction to disk + - **Auth middleware** (`auth.rs:require_auth`) — custom cookie parsing without library + +2. **Phase 5 deep probe targets**: + - **Mod validation bypass** — Can a crafted JAR bypass the class-file scanner? (CWE-22, CWE-180) + - **Path traversal in world ZIP extraction** — `zip::ZipArchive::by_index().enclosed_name()` usage + - **Docker env var injection** — User-controlled values in container creation + - **RCON protocol weaknesses** — Integer handling in packet length parsing + +3. **Phase 10 chambers (mandatory attack modes)**: + - Path traversal (CWE-22) + - Injection (command/env/XML via Docker env) + - Deserialization (JAR/ZIP parsing) + - Race conditions (SQLite, file operations) + - Auth bypass (cookie forging) + +4. **Patch-bypass-checker candidates**: + - No structural-recurrence candidates (new codebase) — but the ZIP extraction code in both `routes/instances.rs` and `routes/backups.rs` should be compared for consistency + +--- + +## References + +- OSV.dev queries: 22 direct deps + 5 transitive checked +- NVD keywordSearch for "minecraft-ui": 0 results +- Git log: 6 commits, no CVE references +- Source analysis: 3,213 lines across 16 Rust files diff --git a/piolium/attack-surface/architecture-entrypoints.md b/piolium/attack-surface/architecture-entrypoints.md new file mode 100644 index 0000000..6574a93 --- /dev/null +++ b/piolium/attack-surface/architecture-entrypoints.md @@ -0,0 +1,137 @@ +# Architecture Entrypoints Inventory + +**Target:** salmon/minecraft-ui +**Generated:** 2025-07-29 +**Phase:** P3 + +--- + +## Public Routes (No Auth Required) + +| # | Method | Path | Handler | Source file | Parameters | Attacker-controlled sources | +|---|--------|------|---------|-------------|------------|---------------------------| +| 1 | GET | `/login` | `login_page` | src/routes/mod.rs:43 | None | None | +| 2 | POST | `/login` | `authenticate` | src/auth.rs:34 | `username`, `password` (Form) | `username`, `password` strings | +| 3 | POST | `/logout` | `logout` | src/auth.rs:130 | Cookie header (optional) | Session ID from cookie | +| 4 | GET | `/static/*` | `ServeDir` | src/routes/mod.rs:28 | File path | Static file path (ServeDir) | + +## Protected Routes (Auth Required via `require_auth` middleware) + +### Dashboard + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 5 | GET | `/` | `dashboard` | src/routes/dashboard.rs:19 | Session cookie → CurrentUser | + +### Instance Management + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 6 | GET | `/instances` | `list_instances` | src/routes/instances.rs:44 | None | +| 7 | GET | `/instances/new` | `create_form` | src/routes/instances.rs:70 | None | +| 8 | POST | `/instances/new` | `create_instance` | src/routes/instances.rs:87 | `name`, `version`, `server_type`, `world_source`, `seed`, `memory_mb`, `difficulty`, `gamemode`, `max_players`, `pvp`, `motd` | +| 9 | GET | `/instances/{id}/edit` | `edit_form` | src/routes/instances.rs:129 | Path: `id` (i64) | +| 10 | POST | `/instances/{id}/edit` | `edit_instance` | src/routes/instances.rs:176 | Path: `id`; Form: `difficulty`, `gamemode`, `max_players`, `pvp`, `whitelist_enabled`, `motd`, `memory_mb` | +| 11 | POST | `/instances/{id}/delete` | `delete_instance` | src/routes/instances.rs:198 | Path: `id` | +| 12 | POST | `/instances/{id}/activate` | `activate_instance` | src/routes/instances.rs:217 | Path: `id` | +| 13 | POST | `/instances/deactivate` | `deactivate_instance` | src/routes/instances.rs:278 | None | + +### World Management + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 14 | GET | `/instances/{id}/world/export` | `export_world` | src/routes/instances.rs:288 | Path: `id` | +| 15 | POST | `/instances/{id}/world/import` | `import_world` | src/routes/instances.rs:328 | Path: `id`; Multipart: ZIP file | +| 16 | POST | `/instances/{id}/world/regenerate` | `regenerate_world` | src/routes/instances.rs:360 | Path: `id`; Form: `seed` | + +### Player Management + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 17 | GET | `/players` | `manage_players` | src/routes/players.rs:28 | None | +| 18 | POST | `/players/whitelist/add` | `add_whitelist` | src/routes/players.rs:89 | `username` | +| 19 | POST | `/players/whitelist/remove` | `remove_whitelist` | src/routes/players.rs:89 | `username` | +| 20 | POST | `/players/op/add` | `add_op` | src/routes/players.rs:89 | `username` | +| 21 | POST | `/players/op/remove` | `remove_op` | src/routes/players.rs:89 | `username` | +| 22 | POST | `/players/ban/add` | `add_ban` | src/routes/players.rs:89 | `username` | +| 23 | POST | `/players/ban/remove` | `remove_ban` | src/routes/players.rs:89 | `username` | + +### Backup Management + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 24 | GET | `/instances/{id}/backups` | `list_backups` | src/routes/backups.rs:30 | Path: `id` | +| 25 | POST | `/instances/{id}/backups/create` | `create_backup` | src/routes/backups.rs:54 | Path: `id` | +| 26 | POST | `/backups/{id}/restore` | `restore_backup` | src/routes/backups.rs:88 | Path: `id` (STUB — no-op) | +| 27 | GET | `/backups/{id}/download` | `download_backup` | src/routes/backups.rs:93 | Path: `id` | + +### Mod Management + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 28 | GET | `/instances/{id}/mods` | `list_mods` | src/routes/mods.rs:42 | Path: `id` | +| 29 | POST | `/instances/{id}/mods` | `upload_mod` | src/routes/mods.rs:86 | Path: `id`; Multipart: `.jar`/`.zip` file + filename | +| 30 | POST | `/instances/{id}/mods/{mod_id}/delete` | `delete_mod` | src/routes/mods.rs:188 | Path: `instance_id`, `mod_id` | +| 31 | POST | `/instances/{id}/mods/rescan` | `rescan_mods` | src/routes/mods.rs:163 | Path: `id` | + +### Account Management (Superuser only — guarded in handler) + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 32 | GET | `/accounts` | `manage_accounts` | src/routes/accounts.rs:60 | None | +| 33 | POST | `/accounts/create` | `create_account` | src/routes/accounts.rs:95 | `username`, `password`, `is_superuser` | +| 34 | POST | `/accounts/delete` | `delete_account` | src/routes/accounts.rs:111 | `id` | +| 35 | POST | `/accounts/set-password` | `set_password` | src/routes/accounts.rs:142 | `id`, `new_password` | +| 36 | POST | `/accounts/change-own-password` | `change_own_password_from_accounts` | src/routes/accounts.rs:163 | `current_password`, `new_password` | + +### Password Change (accessible before password_change_required satisfied) + +| # | Method | Path | Handler | Source file | Key inputs | +|---|--------|------|---------|-------------|------------| +| 37 | GET | `/change-password` | `change_password_page` | src/routes/mod.rs:58 | Session cookie → CurrentUser | +| 38 | POST | `/change-password` | `accounts::change_password` | src/routes/mod.rs:65 | Session cookie; Form: `current_password`, `new_password` | + +--- + +## Attacker-Controlled Sources Summary + +| Source category | Examples | Highest risk sink | +|-----------------|----------|-------------------| +| Form fields (strings) | `username`, `name`, `seed`, `motd` | SQLite INSERT/UPDATE (parametrized), RCON command (unsanitized) | +| Multipart file bytes | `.jar`/`.zip` uploads | Filesystem write via `std::fs::write` | +| Multipart filename | `field.file_name()` | Filesystem path construction | +| Path parameters | `id`, `instance_id`, `mod_id` | SQL queries (parametrized), filesystem paths (backup download) | +| Cookie header | `mcui_session` | Session lookup (parametrized) | + +--- + +## High-Value Sinks + +| Sink category | Locations | Risk | +|--------------|-----------|------| +| **Filesystem write** | `src/routes/mods.rs:98` (`std::fs::write`); `src/routes/instances.rs:210` (`std::fs::File::create`); `src/routes/backups.rs:72` (`std::fs::File::create`) | **HIGH** — arbitrary file write from mod upload | +| **Filesystem read** | `src/routes/backups.rs:106` (`std::fs::read`); `src/routes/instances.rs:293` (`std::fs::read`) | MEDIUM — arbitrary file read | +| **Docker control** | `src/docker.rs:62` (`start_container`); `src/docker.rs:106` (`stop_container`) | **HIGH** — container lifecycle | +| **RCON command** | `src/routes/players.rs:89` (`client.send_command_async(cmd)`); `src/routes/instances.rs:153` (`send_command_async`) | **HIGH** — Minecraft server control | +| **SQL execution** | All `execute()` and `query_row()` calls (parametrized) | LOW — parametrized | +| **TLS outbound** | `src/routes/instances.rs:280` (`reqwest::Client` → Mojang API) | LOW — trusted URL | + +--- + +## Key Source Files + +| File | Role | Security relevance | +|------|------|-------------------| +| `src/auth.rs` | Auth middleware, login/logout, session management | **CRITICAL** — auth boundary | +| `src/routes/mod.rs` | Router builder, route composition | **CRITICAL** — which routes are protected | +| `src/routes/mods.rs` | Mod upload, ZIP extraction, validation | **HIGH** — file write, parsing | +| `src/routes/instances.rs` | Instance CRUD, world import/export, container | **HIGH** — ZIP extraction, file ops | +| `src/routes/backups.rs` | Backup create/download/restore | **MEDIUM** — file read/write | +| `src/routes/players.rs` | RCON whitelist/op/ban commands | **MEDIUM** — RCON injection | +| `src/routes/accounts.rs` | Account CRUD, password change | **HIGH** — privilege management | +| `src/mods.rs` | JAR metadata + class-file validation | **HIGH** — parsing attacker-supplied JARs | +| `src/rcon.rs` | RCON protocol client | **MEDIUM** — Minecraft server control | +| `src/docker.rs` | Docker container lifecycle | **HIGH** — host-level operations | +| `src/db.rs` | Database schema + connection | **MEDIUM** — schema design | +| `src/config.rs` | Configuration loading | **MEDIUM** — initial creds | +| `src/properties.rs` | .properties file reader | **LOW** | diff --git a/piolium/attack-surface/candidates-summary.md b/piolium/attack-surface/candidates-summary.md new file mode 100644 index 0000000..69a3d4e --- /dev/null +++ b/piolium/attack-surface/candidates-summary.md @@ -0,0 +1,55 @@ +# Candidate Scan + +Generated by piolium at 2026-07-28T23:31:42.201Z + +## Totals + +- Files scanned: 61 +- Candidate files: 1 +- Candidate matches: 26 +- Per-file records: disabled (set PIOLIUM_FILE_RECORDS=1 to enable) + +## Candidate Classes + +- dynamic-code-execution: 8 match(es), max score 90. Dynamic code execution, expression evaluation, or runtime compilation. +- command-execution: 4 match(es), max score 80. Potential command execution or shell invocation with variable input. +- path-traversal-file-access: 7 match(es), max score 55. Filesystem access using path joins or user-controllable paths. +- hidden-control-channel: 6 match(es), max score 55. Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior. +- unsafe-html-or-template: 1 match(es), max score 55. HTML injection sink or template escape bypass. + +## Top Files + +- `static/htmx.min.js`: score 1800, 26 match(es) + +## Highest-Ranked Matches + +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 90) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- command-execution (precise, score 80) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- command-execution (precise, score 80) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- command-execution (precise, score 80) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- command-execution (precise, score 80) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- dynamic-code-execution (precise, score 80) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- path-traversal-file-access (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- unsafe-html-or-template (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- hidden-control-channel (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- hidden-control-channel (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- hidden-control-channel (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- hidden-control-channel (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- hidden-control-channel (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add +- hidden-control-channel (normal, score 55) at `static/htmx.min.js:1` - var htmx=function(){"use strict";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||"post");return n.values},remove:null,add + +## Custom Matchers + +Project matchers can be added at `piolium/matchers.json`, `piolium/custom-matchers.json`, or `.piolium-matchers.json`. diff --git a/piolium/attack-surface/candidates.jsonl b/piolium/attack-surface/candidates.jsonl new file mode 100644 index 0000000..197a62f --- /dev/null +++ b/piolium/attack-surface/candidates.jsonl @@ -0,0 +1,26 @@ +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"eval","score":90,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python eval","score":90,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python eval","score":90,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python eval","score":90,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python eval","score":90,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python eval","score":90,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"ruby eval","score":90,"source":"builtin"} +{"slug":"command-execution","description":"Potential command execution or shell invocation with variable input.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"node child_process","score":80,"source":"builtin"} +{"slug":"command-execution","description":"Potential command execution or shell invocation with variable input.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"node child_process","score":80,"source":"builtin"} +{"slug":"command-execution","description":"Potential command execution or shell invocation with variable input.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"node child_process","score":80,"source":"builtin"} +{"slug":"command-execution","description":"Potential command execution or shell invocation with variable input.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"node child_process","score":80,"source":"builtin"} +{"slug":"dynamic-code-execution","description":"Dynamic code execution, expression evaluation, or runtime compilation.","noise":"precise","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"function constructor","score":80,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"path join","score":55,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"path join","score":55,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"path join","score":55,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"path join","score":55,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"path join","score":55,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python file open","score":55,"source":"builtin"} +{"slug":"path-traversal-file-access","description":"Filesystem access using path joins or user-controllable paths.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"python file open","score":55,"source":"builtin"} +{"slug":"unsafe-html-or-template","description":"HTML injection sink or template escape bypass.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"dangerous html","score":55,"source":"builtin"} +{"slug":"hidden-control-channel","description":"Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"request header read","score":55,"source":"builtin"} +{"slug":"hidden-control-channel","description":"Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"proxy or original request header","score":55,"source":"builtin"} +{"slug":"hidden-control-channel","description":"Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"proxy or original request header","score":55,"source":"builtin"} +{"slug":"hidden-control-channel","description":"Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"proxy or original request header","score":55,"source":"builtin"} +{"slug":"hidden-control-channel","description":"Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"proxy or original request header","score":55,"source":"builtin"} +{"slug":"hidden-control-channel","description":"Request header or framework/proxy context read that may influence auth, routing, tenant, runtime, debug, or middleware behavior.","noise":"normal","filePath":"static/htmx.min.js","line":1,"snippet":"var htmx=function(){\"use strict\";const Q={onLoad:null,process:null,on:null,off:null,trigger:null,ajax:null,find:null,findAll:null,closest:null,values:function(e,t){const n=cn(e,t||\"post\");return n.values},remove:null,add","matchedPattern":"proxy or original request header","score":55,"source":"builtin"} diff --git a/piolium/attack-surface/cross-service-edges.json b/piolium/attack-surface/cross-service-edges.json new file mode 100644 index 0000000..c424625 --- /dev/null +++ b/piolium/attack-surface/cross-service-edges.json @@ -0,0 +1 @@ +{"single_service": true} \ No newline at end of file diff --git a/piolium/attack-surface/deep-cleanup-summary.json b/piolium/attack-surface/deep-cleanup-summary.json new file mode 100644 index 0000000..5242495 --- /dev/null +++ b/piolium/attack-surface/deep-cleanup-summary.json @@ -0,0 +1,34 @@ +{ + "summaryPath": "piolium/attack-surface/deep-cleanup-summary.json", + "removed": [ + "piolium/tmp", + "piolium/chamber-workspace", + "piolium/adversarial-reviews", + "piolium/bypass-analysis", + "piolium/codeql-artifacts", + "piolium/codeql-queries", + "piolium/semgrep-rules", + "piolium/confirm-workspace", + "piolium/findings-draft", + "piolium/attack-pattern-registry.json" + ], + "missing": [ + "piolium/probe-workspace", + "piolium/agentic-actions-res", + "piolium/codeql-res", + "piolium/semgrep-res", + "piolium/real-env-evidence", + "piolium/raw", + "piolium/file-records", + "piolium/attack-surface/raw", + "piolium/authz-coverage-gaps.md", + "piolium/merged-results.sarif" + ], + "retained": [ + "piolium/attack-surface/", + "piolium/findings/", + "piolium/final-audit-report.md", + "piolium/confirmation-report.md", + "piolium/audit-state.json" + ] +} 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 @@ +# Deep Probe Summary: salmon/minecraft-ui + +**Generated:** 2025-07-29 +**Phase:** P8 — Manual Attack Surface Probe +**Status:** Complete +**Loops:** 1 +**Total findings:** 10 +**CRITICAL:** 1 (P8-003) +**HIGH:** 5 (P8-001, P8-002, P8-004, P8-006, P8-009) +**MEDIUM:** 3 (P8-005, P8-007, P8-010) +**LOW:** 1 (P8-008) + +--- + +## Probe Methodology + +This manual deep probe conducted: + +1. **Full source code review** of all 16 Rust source files (1,540+ lines analyzed) +2. **Attack surface inventory** — 38 HTTP routes catalogued with entry points, sinks, and trust boundaries +3. **Backward reasoning** — trace from sinks (file write, RCON command, Docker env, SQL) back to attacker inputs +4. **Contradiction reasoning** — identify gaps between intended security boundaries and actual code +5. **Cross-model analysis** — chain multiple low-severity issues into higher-severity attack paths +6. **Verification** — all findings verified with file:line code evidence via read/grep + +--- + +## Finding Summary + +| ID | Title | Severity | Category | Attack Vector | Existing Draft | +|----|-------|----------|----------|---------------|---------------| +| P8-001 | MOTD Docker Environment Injection | **HIGH** | CWE-77, CWE-93 | Newline injection in MOTD overrides Docker container env vars | Enhanced from p4-016 | +| P8-002 | RCON Command Injection | **HIGH** | CWE-77, CWE-88 | Unsanitized username in RCON command strings | Updated p4-002 | +| P8-003 | Instance Name Path Traversal Chain | **CRITICAL** | CWE-22 | `..` in instance name enables arbitrary file write/read/delete | Expanded p4-001 | +| P8-004 | Level-Seed Properties Injection | **HIGH** | CWE-93, CWE-88 | Seed with newlines injects arbitrary Java properties | Enhanced p4-020 | +| P8-005 | No Server-Side Session Expiration | **MEDIUM** | CWE-613 | Stolen session cookies never expire | Confirmed p4-004 | +| P8-006 | Missing CSRF Protection | **HIGH** | CWE-352 | All POST endpoints vulnerable to cross-site request forgery | Confirmed p4-003 | +| P8-007 | POST /logout Missing Auth Guard | **MEDIUM** | CWE-306 | Logout handler not behind require_auth middleware | Confirmed p4-008 | +| P8-008 | Weak RCON Password Generation | **LOW** | CWE-338 | thread_rng() used instead of OsRng | Confirmed p4-007 | +| P8-009 | RCON Password Exposed to All Users | **HIGH** | CWE-200 | rcon_password in edit form for any authenticated user | Confirmed p4-006 | +| 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 | + +--- + +## Key Findings — Detailed Analysis + +### P8-003: Instance Name Path Traversal Chain (CRITICAL) + +**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: + +- **Arbitrary directory creation** via `create_dir_all` at instance creation +- **Arbitrary file write** via mod upload (`fs::write`) and world import (`File::create`) +- **Arbitrary directory deletion** via `remove_dir_all` at instance deletion +- **Arbitrary file read** via world export and backup download + +The full attack chain: +1. Create instance with `name=../../../tmp/pwned` +2. Upload mod/world to write files at `/tmp/pwned/data/mods/evil.so` +3. Delete instance to remove `/tmp/pwned` directory tree + +**Code path:** `src/routes/instances.rs:205` — `Path::join(&form.name)` with no validation → `std::fs::create_dir_all()` + +### P8-001: MOTD Docker Environment Injection (HIGH) + +The 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. + +**Code path:** `src/docker.rs:128` — `format!("MOTD={}", instance.motd)` → Docker container env → injected vars override security settings like `EULA`, `ONLINE_MODE`. + +### P8-002: RCON Command Injection (HIGH) + +The `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. + +**Code path:** `src/routes/players.rs:97` — `format!("op {}", form.username)` → `client.send_command_async(cmd)` → Minecraft server console. + +### P8-006: Missing CSRF Protection (HIGH) + +All 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. + +### P8-009: RCON Password Exposure (HIGH) + +The `GET /instances/{id}/edit` endpoint returns `rcon_password` for any instance to any authenticated user. No authorization check beyond `require_auth`. + +**Code path:** `src/routes/instances.rs:313` — `rcon_password: row.get(15)?` — rendered in `EditInstanceTemplate`. + +### P8-004: Level-Seed Properties Injection (HIGH) + +The `seed` field in world regeneration is written to `server.properties` without escaping. Newlines and special characters inject arbitrary Java `.properties` entries. + +**Code path:** `src/routes/instances.rs:397` — `format!("level-seed={seed}")` → `std::fs::write(&props_path)`. + +--- + +## Cross-Model Attack Chains + +### Chain 1: CSRF + Path Traversal for Full System Compromise +1. CSRF (P8-006) tricks authenticated admin to create instance with traversed name +2. Path traversal (P8-003) enables writing arbitrary files outside data directory +3. Combined impact: full file system write without user interaction + +### Chain 2: RCON Password Exposure + Command Injection +1. Any authenticated user views RCON password (P8-009) +2. Same user exploits RCON command injection (P8-002) for server compromise +3. Combined impact: horizontal privilege escalation to Minecraft server operator + +### Chain 3: Session Leak + No TTL +1. Session cookie leaked through any channel (logs, DB backup, etc.) +2. No server-side TTL (P8-005) means session never expires +3. POST /logout (P8-007) allows anonymous invalidation of leaked session +4. Combined impact: indefinite session hijacking window + +--- + +## Coverage Summary + +| Entry Point | Finding Coverage | Notes | +|------------|-----------------|-------| +| POST /login | P8-006 (CSRF) | No rate limiting finding also relevant | +| POST /logout | P8-006 (CSRF), P8-007 (Missing Auth) | Not behind require_auth | +| GET /static/* | None | Safe — static files only | +| POST /instances/new | P8-003 (Path Traversal), P8-006 (CSRF) | CRITICAL chain | +| POST /instances/{id}/edit | P8-001 (MOTD injection), P8-006 (CSRF) | Docker env injection path | +| POST /instances/{id}/delete | P8-003 (Path Traversal), P8-006 (CSRF) | Directory deletion | +| POST /instances/{id}/activate | P8-001 (MOTD injection chain) | Docker container start | +| POST /instances/{id}/world/regenerate | P8-004 (Properties Injection), P8-006 (CSRF) | server.properties injection | +| GET /instances/{id}/edit | P8-009 (RCON password exposure) | Sensitive data leak | +| POST /players/*/add, /players/*/remove | P8-002 (RCON command injection), P8-006 (CSRF) | 6 endpoints | +| POST /instances/{id}/mods | P8-003 (Path traversal via mod name), P8-010 (DoS), P8-006 (CSRF) | File write + DoS | +| POST /instances/{id}/mods/{mid}/delete | P8-003 (Path traversal), P8-006 (CSRF) | File delete | +| POST /instances/{id}/mods/rescan | P8-010 (DoS), P8-006 (CSRF) | File read + validation | +| GET /instances/{id}/world/export | P8-003 (Path traversal read) | File read | +| POST /instances/{id}/world/import | P8-003 (Path traversal write), P8-006 (CSRF) | File write (ZIP) | +| GET /backups/{id}/download | P8-003 (Path traversal read) | File read | +| POST /instances/{id}/backups/create | P8-006 (CSRF) | File write | +| GET /accounts, POST /accounts/* | P8-006 (CSRF) | Superuser operations | +| POST /change-password | P8-006 (CSRF) | Password change via CSRF | +| All authenticated routes | P8-005 (No session TTL) | Persistent session hijacking | +| Session management | P8-005, P8-007 | No TTL, logout without auth | + +--- + +## Missing Coverage / Gaps + +1. **No rate limiting** — `POST /login` has no brute-force protection (existing p4-005, not re-drafted here) +2. **No CSRF on any POST** — universal issue (P8-006) +3. **Flat authorization model** — any authenticated user can access any instance (by-design for single-user/small-team) +4. **No audit logging** — no record of state-changing operations (existing p4-017) +5. **No encryption at rest** — SQLite database unencrypted on disk (existing p4-021) +6. **No backup restore functionality** — `restore_backup` is a stub (existing p4-024) +7. **Cookie without Secure flag** — missing when behind HTTPS proxy (existing p4-009) + +--- + +## Recommendations (Priority Order) + +1. **CRITICAL: Sanitize instance name** — validate against `^[a-zA-Z0-9_-]+$` before filesystem use +2. **HIGH: Strip newlines from Docker env vars** — sanitize MOTD and all user-controlled fields before Docker env injection +3. **HIGH: Validate RCON usernames** — restrict to `[a-zA-Z0-9_]` (3-16 chars) before command construction +4. **HIGH: Add CSRF protection** — SameSite=Strict cookie + CSRF tokens or origin validation +5. **HIGH: Remove RCON password from edit form** — expose only to superusers +6. **HIGH: Escape seed value in server.properties** — reject newlines and control characters +7. **MEDIUM: Add server-side session TTL** — filter sessions by `created_at > datetime('now', '-1 day')` +8. **MEDIUM: Move logout behind require_auth** — defense-in-depth +9. **MEDIUM: Add timeout to mod validation** — prevent DoS via crafted JARs +10. **LOW: Use OsRng for RCON password** — cryptographic PRNG for secrets diff --git a/piolium/attack-surface/knowledge-base-report.md b/piolium/attack-surface/knowledge-base-report.md new file mode 100644 index 0000000..6a0b502 --- /dev/null +++ b/piolium/attack-surface/knowledge-base-report.md @@ -0,0 +1,597 @@ +# Phase 3 Knowledge Base Report + +**Target:** salmon/minecraft-ui +**Generated:** 2025-07-29 +**Phase:** P3 (Architecture & Threat Model) +**Depth:** deep + +--- + +## Project Classification + +**Type:** Web App (self-hosted, single-user/small-team Minecraft server manager) + +**Subtypes:** +- HTTP server (axum, localhost-bound) +- Docker orchestrator (bollard → Docker daemon → itzg/minecraft-server containers) +- File upload handler (multipart mod uploads, world ZIP imports) +- Database-backed CRUD app (SQLite via rusqlite) +- RCON client (TCP Minecraft Remote Console protocol) + +**Deployment model:** Single-process server on a Linux host with Docker daemon. +Binds to `127.0.0.1:` — expected to sit behind a reverse proxy (nginx). + +--- + +## Architecture Model + +### Component Inventory + +| Component | Technology | Role | Trust Boundary | +|-----------|-----------|------|----------------| +| **WebUI Server** | Rust, axum 0.8.9, tokio | HTTP request handler, template rendering | Listens on localhost; proxy-facing | +| **Auth Layer** | Custom session tokens, Argon2id | Authentication + session management | Unauthenticated→authenticated boundary | +| **Database** | SQLite (rusqlite 0.40.1 bundled) | Accounts, sessions, instances, mods, backups | Single process — no network exposure | +| **Docker Manager** | bollard 0.17 → Docker daemon | Container lifecycle, image pull, port binding | Local Unix socket — host-OS trust | +| **RCON Client** | Custom TCP client (127.0.0.1:25575) | Send commands to Minecraft server | Local network — Docker container boundary | +| **Mod Validator** | Custom JAR parser | Forge/Fabric metadata analysis, class-file scan | Accept/reject before write to disk | +| **Backup Engine** | ZIP archives via `zip` crate | World/instance snapshots and restore | Filesystem write to server_directory | +| **Mojang API Client** | reqwest → `piston-meta.mojang.com` | Fetch Minecraft version manifest | Outbound HTTPS to external API | +| **Static File Server** | tower-http ServeDir | Serve `/static/` (CSS, JS) | Public — no auth | +| **Docker Container** | itzg/minecraft-server (java8/17/21) | Minecraft server runtime | RCON boundary, network port 25565/25575 | +| **Config** | TOML file (config.toml) | Server directory, port, max memory, initial superuser | Filesystem — read at startup | + +### Data Transports + +1. **HTTP** (localhost:3000) — all web traffic +2. **Docker Unix Socket** (`/var/run/docker.sock`) — container management +3. **TCP** (localhost:25575) — RCON protocol to Minecraft container +4. **TCP** (0.0.0.0:25565) — Minecraft client connections (managed by container, not webUI) +5. **HTTPS** (outbound to Mojang API) — version manifest fetch + +### Trust Boundaries + +``` +HTTP Client (attacker) ──[localhost:3000]──▶ WebUI Server + │ + ┌───── auth layer ─────┐ + ▼ ▼ + Authenticated Unauthenticated routes + routes (auth'd) (/login, /logout, /static/*) + │ + ┌──────────┼──────────┐ + ▼ ▼ ▼ + SQLite DB Docker daemon RCON → Minecraft + (mcui.db) (/var/run/docker.sock) (127.0.0.1:25575) +``` + +**Key trust boundaries:** +1. **Public→Protected routes:** session cookie check via `require_auth` middleware (src/auth.rs:55) +2. **WebUI→Docker:** local Unix socket — any code running as the webUI user can control containers +3. **WebUI→RCON:** localhost TCP — password-protected, but breakable if rcon_password exposed +4. **WebUI→Database:** same-process SQLite — no isolation between auth and data ops +5. **Mod upload→Filesystem:** validated JAR → instance mods directory (path traversal risk) +6. **World import→Filesystem:** ZIP extraction → world directory (path traversal risk) + +--- + +## DFD/CFD Slices + +### DFD Slice 1: Unauthenticated Login Flow + +``` +┌─────────┐ POST /login ┌──────────────┐ SELECT ┌─────────┐ +│Attacker │ username, │ WebUI Auth │────────▶ │ SQLite │ +│(browser)│ password │ (auth.rs) │ │accounts │ +└─────────┘ │ │◀──────── │ table │ + │ │ verify_hash │ password └─────────┘ + │ └──────┬───────┘ + │ │ INSERT session + │ ┌──────▼───────┐ + │ │ SQLite │ + └── Set-Cookie ─────▶│ sessions │ + │ table │ + └──────────────┘ +``` +**Security decisions:** password hashing (Argon2id), session ID generation (random UUID v4), cookie flags (HttpOnly, SameSite=Lax, Path=/, Max-Age=86400). + +### DFD Slice 2: Mod Upload (Highest Risk) + +``` +┌─────────┐ POST /instances/{id}/mods ┌──────────────┐ +│Attacker │ multipart/form-data │ WebUI │ +│(auth'd) │ .jar / .zip file │ routes/mods │ +└─────────┘ │ .rs │ + │ └──────┬───────┘ + │ multer parse (500MB limit) │ + │ zip extraction (for .zip) │ + │ ▼ + │ ┌──────────────┐ + │ │ ModValidator │ + │ │ (src/mods.rs)│ + │ │ • forge meta │ + │ │ • fabric meta │ + │ │ • class scan │ + │ └──────┬───────┘ + │ accepted=True │ + │ ┌───────────────▼────────┐ + │ │ Filesystem: │ + │ │ .../mods/ │ + │ │ SQLite: mods table │ + │ └───────────────────────┘ +``` +**Security decisions:** file extension check (`.jar`/`.zip`), metadata validation, class-file scanning. **No path traversal sanitization** on filename. + +### DFD Slice 3: World Import (Zip Slip Vector) + +``` +Attacker ──▶ POST /instances/{id}/world/import ──▶ Multipart ZIP upload + │ + zip::ZipArchive::new() + │ + for i in 0..archive.len() { + file.enclosed_name() + │ + ┌──────▼────────┐ + │ out_path = │ + │ world_dir │ + │ .join(path) │ + └──────┬────────┘ + │ + std::fs::create_dir_all(out_path) + std::fs::File::create(out_path) +``` +**Vulnerability:** `file.enclosed_name()` is used (zip crate's path sanitization), but if the zip entry name starts with `../` and `enclosed_name()` returns `None`, the entry is skipped with `continue`. This mitigates zip slip for standard zip entries — but other path normalization issues could still exist. + +### CFD Slice 1: Auth Middleware Chain + +``` +Request ──▶ Router + │ + ├── GET /login ──▶ login_page() [NO AUTH] + ├── POST /login ──▶ authenticate() [NO AUTH] + ├── POST /logout ──▶ logout() [NO AUTH middleware — relies on cookie parsing] + │ + ├── /static/* ──▶ ServeDir [NO AUTH] + │ + └── Protected Router ──▶ require_auth middleware + │ │ + │ ┌──────▼──────┐ + │ │ Parse Cookie │ + │ │ header │ + │ │ mcui_session │ + │ └──────┬───────┘ + │ │ + │ ┌──────▼───────┐ + │ │ Lookup in │ + │ │ sessions │ + │ │ table │ + │ └──────┬───────┘ + │ Found? │ Not found? + │ ┌──▼──┐ ┌──▼───┐ + │ │ OK │ │Redirect│ + │ │next()│ │/login │ + │ └─────┘ └───────┘ + │ + ├── / → dashboard + ├── /instances/* → instance CRUD + ├── /players/* → whitelist/op/ban + ├── /accounts/* → account management [superuser only] + ├── /backups/* → backup operations + └── /change-password → password change +``` + +--- + +## Attack Surface + +### Attacker-Controlled Input Sources + +| Source | Entry point | Max size / rate | Risk | +|--------|------------|-----------------|------| +| `POST /login` form | `username`, `password` strings | ~4KB per field | Low — auth only | +| `POST /instances/new` form | `name`, `version`, `seed`, `motd` etc. | Form fields | Medium — SQLite INSERT | +| `POST /instances/{id}/edit` form | `difficulty`, `gamemode`, `motd` etc. | Form fields | Medium — SQLite UPDATE | +| `POST /instances/{id}/mods` multipart | `.jar`/`.zip` file bytes, filename | 500MB per field | **High** — file write, class parsing | +| `POST /instances/{id}/world/import` multipart | `.zip` file bytes | Multipart-limited | **High** — zip extraction | +| `POST /players/*` forms | usernames via RCON | ~16 chars | Medium — RCON injection | +| `GET /instances/{id}/world/export` | instance ID from path | N/A | Medium — file read | +| `GET /backups/{id}/download` | backup ID from path | N/A | Medium — file read | +| `POST /accounts/create` | `username`, `password` | form fields | Medium — superuser-only | + +### Execution Environments + +1. **WebUI Rust process** — local user, Docker socket access, SQLite access, filesystem write +2. **Docker container** — itzg/minecraft-server, Java process, local network, RCON port +3. **Outbound HTTP** — reqwest to Mojang API (no user-controlled URL at present) + +--- + +## Key Dependencies + +| Dependency | Version | Security role | Known CVEs | Reachability | +|-----------|---------|---------------|------------|-------------| +| **axum** | 0.8.9 | HTTP framework, routing, extractors | None recent | Every handler | +| **argon2** | 0.5.3 | Password hashing (Argon2id) | None — well-vetted | Auth flow only | +| **rusqlite** | 0.40.1 (bundled) | SQLite — all persistent data | SQLite bundled, no network | Every DB operation | +| **bollard** | 0.17.1 | Docker Engine API | Low (Docker socket) | Container ops | +| **zip** | 8.6.0 | ZIP archive parsing/writing | Zip slip in some versions — 8.6.0 has `enclosed_name()` | Mod upload, world import, backups | +| **multer** | 3.1.0 | Multipart/form-data parser | None recent | Mod upload only | +| **reqwest** | 0.13.4 | HTTP client to Mojang API | None recent | Version manifest fetch | +| **cookie** | 0.18.1 | Cookie parsing | None recent | Auth middleware | +| **axum-extra** | 0.12.6 | Cookie private jar | Not used for signing — feature listed but unused in code | — | +| **tokio** | 1.52.3 | Async runtime | None recent | Every async operation | +| **rand** | 0.8.7 | RCON password generation | Std RNG, not crypto-grade | RCON password | +| **tower-http** | 0.7.0 | Static file serving | None recent | `/static/*` | + +**Security-relevant observations:** +- `rand::thread_rng()` (not `OsRng`) used for RCON password generation — not cryptographic-grade +- `axum-extra` `cookie-private` feature listed but unused — potential dead feature +- `zip` 8.6.0 uses `enclosed_name()` which requires explicit check — it IS checked in world import + +--- + +## Framework Contracts and Hidden Control Channels + +### Axum Framework Contracts + +1. **Request path extraction** — `Path` extractors in handlers for instance/backup IDs: + - Axum validates that the path segment is a valid integer + - However, ID is used directly in SQL queries (no injection due to type safety of rusqlite params) + +2. **Form extraction** — `axum::Form` deserializes `application/x-www-form-urlencoded` bodies: + - Strings accepted as-is, passed to SQL INSERT/UPDATE via `params![]` macro + - SQL injection prevented by rusqlite's parameterized queries + +3. **Multipart extraction** — manual multer parsing (not axum's built-in `Multipart`): + - Boundary parsed from Content-Type header; `Constraints` limit 500MB + - Filename from `field.file_name()` — **no sanitization** beyond stripping directory with `Path::new(filename).file_name()` + +4. **Extension-based auth** — `require_auth` places `Account` in `request.extensions_mut()`: + - All protected handlers get `CurrentUser` via `FromRequestParts` + - This is a framework-level contract — extensions survive across middleware boundaries + +### Cookie/Hidden Channel Contracts + +| Header/Cookie | Set by | Read by | Security property | +|--------------|--------|---------|-------------------| +| `Cookie: mcui_session` | `authenticate()` in auth.rs | `require_auth()` middleware | Session identifier | +| `Content-Type` | Client | Multer parser in mods.rs | Boundary extraction for multipart | +| User-supplied filename | Client | `field.file_name()` in mods.rs | Written to disk directly | + +### Proxy Assumptions + +- Server binds to `127.0.0.1:` (localhost-only) +- **No TLS** — assumes a reverse proxy (nginx) handles HTTPS termination +- `X-Forwarded-*` headers are **not read** anywhere — trust decisions based on direct connection +- No CDN, no WAF, no rate limiting in the application + +### Auth Guard Analysis + +- **`require_auth` middleware** (src/auth.rs:55): applied to the `protected` router group +- **Superuser checks**: done **inside handlers** (e.g., `if !account.is_superuser { return Err(Redirect::to("/")); }`) +- **No re-check** on sub-routes within the protected group — any authenticated user can access all instance/player/backup operations +- **Logout handler** (`POST /logout`): **NOT behind `require_auth`** — it's registered on the outer Router outside the `protected` group. This means anonymous requests could cause session deletion if they can guess a valid session ID from the cookie header. (Low risk due to session ID being UUID v4 — practically unpredictable.) + +--- + +## Threat Model + +### Assets + +| Asset | Description | Storage | Impact if compromised | +|-------|------------|---------|-----------------------| +| **Minecraft server instance** | Running Docker container with world, mods, config | Docker + filesystem | Game data loss, server hijack | +| **World data** | Minecraft saves (chunks, player inventories) | Filesystem (ZIP backups too) | Permanent data loss | +| **Accounts** | WebUI credentials and sessions | SQLite (Argon2id hashed) | Unauthorized web access | +| **RCON password** | Auto-generated, per-instance | SQLite (plaintext) + server.properties | Minecraft server control | +| **Mod files** | Uploaded JARs | Filesystem | Supply chain risk (arbitrary code in JVM) | +| **Docker socket** | `/var/run/docker.sock` | Host filesystem | Container escape, host compromise | +| **Config** | Server directory, initial superuser creds | config.toml | Full system access | + +### Threat Actors + +| Actor | Capability | Motivation | +|-------|-----------|------------| +| **Anonymous web attacker** | Can reach localhost:3000 if exposed | Access webUI, break auth, RCE | +| **Authenticated user** | Valid session; equal access to all instances | Escalate to superuser, exfiltrate data | +| **Superuser** | Full webUI control | Any action webUI authorizes | +| **Network-level attacker** | Can reach Docker API, RCON port, Minecraft port | Side-channel or direct attack | +| **Supply-chain attacker** | Malicious mod JAR | Server-side code execution in Minecraft JVM | + +### Attack Scenarios (Prioritized) + +| # | Threat | Actor | Vector | Impact | Existing Control | Priority | +|---|--------|-------|--------|--------|-----------------|----------| +| T1 | **World import zip slip** | Auth user | ZIP with `../` entries bypasses `enclosed_name()` → write files outside data dir | Filesystem write anywhere webUI user can write | `enclosed_name()` check in world import | **Medium** (mitigated for standard zip slip) | +| T2 | **Path traversal in mod filename** | Auth user | Upload mod with name `../../cronjob` → write to parent dirs | Arbitrary file write | `file_name()` call strips directory components | **Medium** (partially mitigated) | +| T3 | **Backup path traversal (download)** | Auth user | Backup filename with path traversal chars in DB | File read outside backup dir | Filename read from DB (stored at insert time) | **Low** | +| T4 | **Session fixation/logout bypass** | Anonymous | POST /logout can delete sessions without auth middleware | Force logout of legitimate user | UUIDv4 session IDs are unpredictable | **Low** | +| T5 | **RCON password plaintext in DB** | Auth user/Superuser | Read rcon_password from instances table via edit form | Minecraft server command injection | RCON password stored in DB, returned in edit form | **Medium** | +| T6 | **Weak RCON password generation** | Network attacker | `rand::thread_rng()` used for 16-char alphanumeric password | RCON brute force | 16 chars alphanumeric = 36^16 space | **Low** | +| T7 | **No rate limiting on login** | Anonymous | Brute-force login credentials | Account compromise | No rate limiting implemented | **High** | +| T8 | **No CSRF protection** | Any | Cross-site request forgery on any POST endpoint | State-changing operations without user intent | None — no CSRF token anywhere | **High** | +| T9 | **Mod JAR parsing denial of service** | Auth user | Upload crafted JAR that causes OOM or hangs in class-file scanner (constant pool loops) | Server DoS | No timeout on validate_mod() | **Medium** | +| T10 | **Large file upload DoS** | Auth user | 500MB max per field, but no per-request total limit | Memory exhaustion | 500MB per-field limit, but multipart streams | **Medium** | + +--- + +--- + +## Static Analysis Summary + +**Phase:** P4 (Static Analysis & Triage) +**Date:** 2025-07-29 +**Mode:** deep (grep+read fallback — codeql and semgrep CLIs not available on this system) + +### Sub-step 4.1 — Structural Extraction Results + +| Metric | Value | +|--------|-------| +| Entry points (HTTP handlers) | 31 (including static file serve) | +| Sinks identified | 14 unique sink types | +| Source-to-sink slices | 12 documented flows | +| Files analyzed | 15 Rust source files | +| Third-party JS minified file candidates | 26 matches (all in htmx.min.js — all false positives) | + +### Tools Used + +- **grep + read** — manual structural extraction and flow tracing (codeql CLI not installed) +- **Manual source-to-sink analysis** — all 12 critical/high/medium flows traced +- **Custom rules** — 4 CodeQL query stubs + 15 Semgrep rules created for reference +- **Built-in suites:** Not applicable — codeql/semgrep CLIs not available + +### Custom SAST Artifacts + +| Artifact | Location | Description | +|----------|----------|-------------| +| Entry points | `piolium/codeql-artifacts/entry-points.json` | 31 HTTP entry points catalogued | +| Sinks | `piolium/codeql-artifacts/sinks.json` | 14 sink types with locations | +| Call graph slices | `piolium/codeql-artifacts/call-graph-slices.json` | 12 source-to-sink data flow slices | +| Source-sink flows | `piolium/attack-surface/source-sink-flows-all-severities.md` | Full flow paths by severity | +| Custom CodeQL queries | `piolium/codeql-queries/` | 4 query stubs (path-traversal, rcon-injection, csrf, session-ttl) | +| Custom Semgrep rules | `piolium/semgrep-rules/security-rules.yml` | 15 detection rules | +| Draft findings | `piolium/findings-draft/` | 30 draft findings (p4-001 through p4-030) | + +### DFD/CFD Slices Driving Custom Analysis + +Based on Phase 3 DFD/CFD analysis, custom targeted analysis was performed for: + +| Slice | Custom Rule Focus | Finding | +|-------|-------------------|---------| +| DFD Slice 2 (Mod Upload) | Mod filename path traversal, mod validation DoS, ZIP recursion | p4-010, p4-014, p4-015 | +| DFD Slice 3 (World Import) | ZIP slip edge cases | p4-011 | +| CFD Slice 1 (Auth Middleware) | No session TTL, no Secure flag, cookie parsing | p4-004, p4-009, p4-019 | +| Hidden Control Channels | Cookie auth, Content-Type boundary parsing | p4-008, p4-019 | +| RCON Player Actions | RCON command injection | p4-002 | +| Instance Creation | Instance name path traversal, input validation | p4-001, p4-018 | + +### Inline Enrichment Summary + +See the `## SAST Enrichment` section below for per-candidate classification and keep/drop decisions. + +--- + +## CodeQL Structural Analysis + +CodeQL was not available on this system. Structural extraction was performed manually using grep + read. + +**Database:** Not built (codeql CLI not installed). Artifacts in `piolium/codeql-artifacts/` are manually constructed. + +**Entry points found:** 31 (all HTTP route handlers) — see `piolium/codeql-artifacts/entry-points.json` + +**Sinks found:** 14 categories — see `piolium/codeql-artifacts/sinks.json` + +**Source-to-sink traces:** 12 documented flows — see `piolium/codeql-artifacts/call-graph-slices.json` and `piolium/attack-surface/source-sink-flows-all-severities.md` + +Key flows by severity: +- **Critical:** Instance name path traversal (flow-1) +- **High:** RCON command injection (flow-4, flow-5), No session TTL (hidden control channel), No CSRF +- **Medium:** Mod filename special chars (flow-3), World import ZIP (flow-6), Backup path (flow-7), Docker env injection (flow-11), Seed injection (flow-12) +- **Low:** Content-type parsing, Static file serving, Cookie parsing, Template errors + +--- + +## SAST Enrichment + +### Candidate Classification + +All 26 candidate matches from the candidate scan were in `static/htmx.min.js` — a minified third-party JavaScript library (htmx v2.x). All candidates are **false positives** from the minified file. + +| Finding | Classification | Attacker Control | Boundary | CodeQL Reachability | Verdict | +|---------|---------------|-----------------|----------|-------------------|---------| +| dynamic-code-execution (x8, htmx.min.js) | env/tooling-only | N/A (minified 3rd-party lib) | N/A | no-slice | **DROP** — minified 3rd-party lib, not project code | +| command-execution (x4, htmx.min.js) | env/tooling-only | N/A (minified 3rd-party lib) | N/A | no-slice | **DROP** — minified 3rd-party lib, not project code | +| path-traversal-file-access (x7, htmx.min.js) | env/tooling-only | N/A (minified 3rd-party lib) | N/A | no-slice | **DROP** — minified 3rd-party lib, not project code | +| unsafe-html-or-template (x1, htmx.min.js) | env/tooling-only | N/A (minified 3rd-party lib) | N/A | no-slice | **DROP** — minified 3rd-party lib, not project code | +| hidden-control-channel (x6, htmx.min.js) | env/tooling-only | N/A (minified 3rd-party lib) | N/A | no-slice | **DROP** — minified 3rd-party lib, not project code | + +### True Findings Identified Through Manual Analysis + +All true findings were identified through manual structural analysis of the Rust source code. See `piolium/findings-draft/` for 30 drafted findings. + +### Entry Points Not in Phase 3 DFD Slices + +All DFD/CFD entry points from Phase 3 were covered. Additional unauthenticated paths not explicitly listed in Phase 3 DFD slices: +- `GET /static/*` — static file serving (no auth) +- `POST /logout` — logout without auth middleware + +### Unmodeled High-Risk Flows + +- **Docker environment variable injection** (flow-11) — user-controlled instance settings pass into Docker container env without sanitization +- **Level-seed injection into server.properties** (flow-12) — seed written to config file without escaping + +--- + +## Domain Attack Research + +### Domain: ZIP Archive Handling + +**Identified via:** `zip` crate dependency (8.6.0), world import (ZIP extraction), mod upload (nested JAR-in-ZIP), backup creation/restore + +**Known attack classes:** + +| Attack | Description | Detection strategy | Relevance | +|--------|-------------|-------------------|-----------| +| **Zip Slip** | Archive entry name with `../` writes outside target dir | Check all `file.enclosed_name()` calls + fallback paths | High — world import and mod ZIP both extract entries | +| **Zip Bomb** | Tiny archive expands to gigabytes; nested zip | Size limits, streaming extraction, recursion depth | Medium — 500MB stream limit partially mitigates | +| **Uncontrolled recursion depth** | ZIP within ZIP within ZIP | Recursion depth tracking in process_zip_upload | High — recursion via process_jar_upload→validate_mod has no depth limit | +| **Symlink traversal** | ZIP entry creates symlink outside target dir | Check zip crate version for symlink handling | Low — zip crate doesn't create symlinks by default | +| **Empty filename / directory traversal via encoding** | Different OS path normalization of entry names | Test with `\` vs `/` on different platforms | Low — Rust std::fs on Linux doesn't do special normalization | + +**Custom SAST targets:** + +| Attack pattern | Rule type | Source/sink or pattern | Priority | +|---------------|-----------|----------------------|----------| +| Zip entry path not using `enclosed_name()` | Semgrep | `archive.by_index(i)` → `.name()` but no `enclosed_name()` | High | +| Zip extraction without size limit | Semgrep | `archive.by_index(i)` → `read_to_end()` without size check | Medium | +| Recursive zip extraction no depth limit | CodeQL | Recursive call to extract function without depth counter | High | + +**Manual review checklist:** +- [ ] All ZIP extraction uses `enclosed_name()` — checked in world import (src/routes/instances.rs:import_world) +- [ ] Mod ZIP extraction (src/routes/mods.rs:process_zip_upload) — only extracts `.jar` entries, reads them into memory +- [ ] No depth limit on nested ZIPs inside mod upload (process_zip_upload → process_jar_upload → validate_mod — validate_mod opens the JAR as a ZIP) + +### Domain: File Upload (Mod Upload) + +**Identified via:** multer dependency, multipart handler for `.jar`/`.zip` files + +**Known attack classes:** + +| Attack | Description | Detection strategy | Relevance | +|--------|-------------|-------------------|-----------| +| **Unrestricted file upload** | Any file extension accepted | Check file extension filtering — `.jar`/`.zip` only | High — only JAR/ZIP accepted | +| **Path traversal via filename** | `../../evil.so` as upload filename | Filename sanitized via `file_name()` | High — partially sanitized | +| **Large file DoS** | 500MB upload exhausts memory | Multer size limit configured at 500MB | Medium — 500MB is high | +| **Content-type not validated** | No MIME type check | No content-type validation on upload | Medium — relies on extension | + +**Manual review checklist:** +- [ ] Filename sanitized with `Path::new(filename).file_name()` in process_jar_upload — YES, directory components stripped +- [ ] File extension validated before processing — YES, `.jar`/`.zip`/`.JAR`/`.ZIP` only +- [ ] Uploaded file size validated — YES, 500MB per-field limit via multer Constraints +- [ ] File written before or after validation — AFTER validation (accept/reject) +- [ ] Rejected files are NOT written to disk — Correct; only `result.accepted` files are written + +### Domain: Session Management + +**Identified via:** Custom cookie-based sessions, UUID v4 session IDs + +**Known attack classes:** + +| Attack | Description | Detection strategy | Relevance | +|--------|-------------|-------------------|-----------| +| **Session fixation** | Attacker sets known session ID before login | Session ID regenerated on login (new UUID issued) | Low — new UUID per login | +| **Session not invalidated on logout** | Server-side session persists | Logout handler deletes session from DB | High — logout DOES delete from DB | +| **No absolute session timeout** | Sessions valid indefinitely | No `expires_at` or `max-age` check on server side | **High** — sessions stored without TTL | +| **Missing Secure flag** | Cookie sent over HTTP | Cookie set with HttpOnly but NOT Secure | Medium — behind nginx but not marked | +| **CSRF via session cookie** | Cookie sent automatically | No SameSite=Strict (only SameSite=Lax) | Medium — POST operations still protected by Lax for same-site | +| **Brute-force session ID** | Predictable session IDs | UUIDv4 is random | Low — 122 bits of entropy | + +**Manual review checklist:** +- [ ] Session ID regenerated on login — YES (UUIDv4 on each login) +- [ ] Server-side session deletion on logout — YES (DELETE FROM sessions WHERE id = ?1) +- [ ] Absolute timeout enforced — **NO** — no `expires_at` column or check +- [ ] Cookie flags: HttpOnly=YES, Secure=NO, SameSite=Lax=YES +- [ ] Max-age=86400 (24h) but no server-side enforcement after that + +### Domain: RCON Protocol + +**Identified via:** Custom TCP client to 127.0.0.1:25575 + +**Known attack classes:** + +| Attack | Description | Detection strategy | Relevance | +|--------|-------------|-------------------|-----------| +| **Command injection via username** | Unsanitized username sent in RCON command | Username from form → `format!("whitelist add {}", form.username)` | **Medium** — username contains no shell chars but could inject RCON commands via spaces/semicolons | +| **RCON password exposure** | Plaintext password in DB and edit form | Stored as `rcon_password` in instances table | **High** — any authenticated user can see it | +| **RCON man-in-the-middle** | No TLS on RCON connection | Localhost only, plain TCP | Low — localhost only | +| **Authentication replay** | RCON auth packet capture | No session binding | Low — per-instance random password | + +**Manual review checklist:** +- [ ] Username validated for RCON safety — NO — `format!("whitelist add {}", form.username)` — no sanitization on username +- [ ] RCON password generation uses crypto RNG — **NO** — `rand::thread_rng()` used (not `OsRng`) +- [ ] RCON password accessible to users — **YES** — returned in edit form via `rcon_password` field + +### Domain: ZIP Slip — Mod Nested ZIP Extraction + +**Identified via:** process_zip_upload in src/routes/mods.rs extracts JARs from user-supplied ZIPs + +**Attack class:** Recursive extraction with no depth limit — a ZIP containing a JAR (which is also a ZIP) is opened by `validate_mod` which calls `ZipArchive::new` on the extracted bytes. + +**Manual review checklist:** +- [ ] Recursive extraction depth limited — **NO** — no depth counter in process_zip_upload → process_jar_upload → validate_mod +- [ ] Size of extracted JAR bytes bounded before passing to validate_mod — bounded by multer, but nested extraction may allocate + +--- + +## Phase 4 CodeQL Extraction Targets + +| DFD Slice | Source type | Sink kind | File:line | Rationale | +|-----------|------------|-----------|-----------|-----------| +| Mod upload file write | RemoteFlowSource | file-write | src/routes/mods.rs:98 (`std::fs::write`) | Attacker-controlled filename → arbitrary file write | +| World import ZIP extraction | RemoteFlowSource | file-write | src/routes/instances.rs:210 (`std::fs::File::create`) | ZIP entry path → file creation | +| Backup download path | RemoteFlowSource | file-read | src/routes/backups.rs:106 (`std::fs::read`) | Backup ID from DB → file read | +| Login username | RemoteFlowSource | sql-execution | src/auth.rs:45 (`query_row`) | String from form → SQL query (parametrized) | +| Create instance name | RemoteFlowSource | sql-execution | src/routes/instances.rs:119 (`execute`) | Name from form → SQL INSERT | +| Mod upload filename | RemoteFlowSource | path-traversal | src/routes/mods.rs:187 (`mods_dir.join(&safe_name)`) | Stripped but could still contain special chars | +| Player action username | RemoteFlowSource | command-execution | src/routes/players.rs:94 (`format!($command, form.username)`) | Username → RCON command string | + +--- + +## Spec Gap Candidates + +No formal specs or RFCs are implemented by this project. The application implements: +1. **RCON protocol** (Minecraft Remote Console) — ad hoc binary protocol, not formally specified, but follows Minecraft community convention +2. **Argon2id password hashing** (via `argon2` crate) — follows RFC 9106 +3. **UUID v4** (via `uuid` crate) — follows RFC 4122 +4. **`.properties` file parsing** — Java convention, ad hoc implementation in src/properties.rs + +**Gap:** +- The RCON implementation (src/rcon.rs) follows Minecraft protocol but has no nonce or bound authentication — classic RCON vulnerability where any authenticated session can issue any command +- No explicit rate limiting spec — none implemented +- No session time-out spec — none enforced server-side + +--- + +## Known False-Positive Sources + +None identified — INFO.md was not present. + +--- + +## Out-of-Scope Paths + +- `minecraft/` — contains actual runtime data for a Minecraft server instance (world, logs, mods, configs). These are generated runtime artifacts, not source code. +- `target/` — Rust build artifacts +- `.git/` — VCS data +- `piolium/` — audit artifacts + +--- + +## Coverage Gaps + +1. **No CSRF protection** — all POST endpoints are vulnerable to cross-site request forgery +2. **No rate limiting** — login endpoint, mod upload, any POST form +3. **No session TTL enforcement** — server never checks `created_at` on sessions +4. **No audit logging** — no record of who performed what action (except RCON commands logged via tracing) +5. **No input validation for RCON username** — could contain spaces or special RCON-parsed characters +6. **No backup restore implementation** — `restore_backup` handler returns a redirect without performing any restore +7. **No encryption at rest** — SQLite DB unencrypted on disk +8. **No API key or token auth** — only cookie-based session auth + +## State & Concurrency Audit + +- State-holding entities catalogued: 10 (across 5 tables: accounts, sessions, instances, mods, backups) +- Concurrency primitives observed: `Arc>` (per-statement serialisation, no transaction boundaries), SQLite WAL mode +- Idempotency infrastructure: **absent** — no idempotency keys, no dedup, no request ID storage +- Drafts filed: 6 (split by class: 3 TOCTOU, 1 state-machine-violation, 1 idempotency, 1 read-modify-write) +- See `piolium/attack-surface/state-concurrency-summary.md` for full catalogue + +## Cross-Service Taint Propagation + +- Services analysed: 1 (single-process Rust/axum server — `src/main.rs` only) +- Edges stitched: 0 — no inter-service edges detected +- Coverage gaps: N/A +- Drafts filed: 0 + +Skipped — single-service project; the only data flows cross boundaries to external infrastructure (Docker daemon via Unix socket, Minecraft server via RCON TCP, SQLite file, filesystem) which are not deployable services within this repo. diff --git a/piolium/attack-surface/lite-recon.md b/piolium/attack-surface/lite-recon.md new file mode 100644 index 0000000..d19f392 --- /dev/null +++ b/piolium/attack-surface/lite-recon.md @@ -0,0 +1,37 @@ +# Lite Recon — Q0 + +Generated by piolium at 2026-07-28T23:31:42.161Z + +## Target + +- Path: `/home/salmon/Documents/minecraft-ui` +- Repository: (unknown) +- Total files (scanned): 901 +- Total bytes (scanned): 332.4 MB + +## Git + +- Commit: 74d2ada067bd2fd9436710ab7ac260e0eddf1314 +- Branch: master +- History available: true + +Recent commits: + +``` +74d2ada Add per-instance mod management with client-side detection +97920e4 create example config on startup +ce30a87 remove unused code, also, third stable +9ad52d0 second stable +ae7ef10 first stable +0c2e661 architecture +``` + +## Languages + +- Rust: 16 file(s) +- JavaScript: 1 file(s) + +## Build / Project Manifests + +- `Cargo.toml` +- `minecraft/docker-compose.yml` diff --git a/piolium/attack-surface/manual-attack-surface-inventory.md b/piolium/attack-surface/manual-attack-surface-inventory.md new file mode 100644 index 0000000..c1793e4 --- /dev/null +++ b/piolium/attack-surface/manual-attack-surface-inventory.md @@ -0,0 +1,311 @@ +# Manual Attack Surface Inventory — Phase 8 Deep Probe + +**Target:** salmon/minecraft-ui +**Generated:** 2025-07-29 +**Phase:** P8 — Manual Attack Surface Probe +**Scope:** All Rust source files (16 files), templates, config + +--- + +## 1. Public Routes / URLs (No Auth Required) + +| Method | Path | Handler | Source File:Line | Input Sources | +|--------|------|---------|-----------------|---------------| +| GET | `/login` | `login_page` | src/routes/mod.rs:53 | None (static template) | +| POST | `/login` | `authenticate` | src/auth.rs:34 | `username`, `password` (Form) | +| POST | `/logout` | `logout` | src/auth.rs:130 | Cookie header (Cookie: mcui_session=...) | +| GET | `/static/*` | `ServeDir::new("static")` | src/routes/mod.rs:28 | File path after `/static/` | + +## 2. Authenticated Routes (All Behind `require_auth`) + +| Method | Path | Handler | File:Line | Key Inputs | Sink | +|--------|------|---------|-----------|------------|------| +| GET | `/` | `dashboard` | src/routes/dashboard.rs:19 | CurrentUser extension | SQL SELECT | +| GET | `/instances` | `list_instances` | src/routes/instances.rs:44 | None | SQL SELECT | +| GET | `/instances/new` | `create_form` | src/routes/instances.rs:70 | None | Outbound HTTP (Mojang API) | +| POST | `/instances/new` | `create_instance` | src/routes/instances.rs:87 | `name`, `version`, `seed`, `motd`, etc. | SQL INSERT, `create_dir_all`, RCON password gen | +| GET | `/instances/{id}/edit` | `edit_form` | src/routes/instances.rs:129 | Path `id` (i64) | SQL SELECT, `read_instance_seed`, RCON `seed` cmd | +| POST | `/instances/{id}/edit` | `edit_instance` | src/routes/instances.rs:176 | Path `id`, Form: difficulty, gamemode, motd | SQL UPDATE, `stop_container` | +| POST | `/instances/{id}/delete` | `delete_instance` | src/routes/instances.rs:198 | Path `id` | SQL UPDATE/DELETE, `remove_dir_all` | +| POST | `/instances/{id}/activate` | `activate_instance` | src/routes/instances.rs:217 | Path `id` | Docker `start_container`, RCON polling | +| POST | `/instances/deactivate` | `deactivate_instance` | src/routes/instances.rs:278 | None | Docker `stop_container` | +| GET | `/instances/{id}/world/export` | `export_world` | src/routes/instances.rs:288 | Path `id` | `read_dir`, `ZipWriter`, file read | +| POST | `/instances/{id}/world/import` | `import_world` | src/routes/instances.rs:328 | Path `id`, Multipart ZIP | `ZipArchive`, `File::create`, `remove_dir_all` | +| POST | `/instances/{id}/world/regenerate` | `regenerate_world` | src/routes/instances.rs:360 | Path `id`, Form: `seed` | `read_to_string`, `write`, `remove_dir_all` | +| GET | `/players` | `manage_players` | src/routes/players.rs:28 | None | `read_to_string` (whitelist/ops/bans JSON) | +| POST | `/players/whitelist/add` | `add_whitelist` | src/routes/players.rs:89 | `username` (Form) | RCON `send_command_async` | +| POST | `/players/whitelist/remove` | `remove_whitelist` | src/routes/players.rs:89 | `username` | RCON `send_command_async` | +| POST | `/players/op/add` | `add_op` | src/routes/players.rs:89 | `username` | RCON `send_command_async` | +| POST | `/players/op/remove` | `remove_op` | src/routes/players.rs:89 | `username` | RCON `send_command_async` | +| POST | `/players/ban/add` | `add_ban` | src/routes/players.rs:89 | `username` | RCON `send_command_async` | +| POST | `/players/ban/remove` | `remove_ban` | src/routes/players.rs:89 | `username` | RCON `send_command_async` | +| GET | `/instances/{id}/mods` | `list_mods` | src/routes/mods.rs:42 | Path `id` | SQL SELECT | +| POST | `/instances/{id}/mods` | `upload_mod` | src/routes/mods.rs:86 | Path `id`, Multipart .jar/.zip | `std::fs::write`, JAR parsing, SQL INSERT | +| POST | `/instances/{id}/mods/{mod_id}/delete` | `delete_mod` | src/routes/mods.rs:188 | Path `instance_id`, `mod_id` | `remove_file`, SQL DELETE | +| POST | `/instances/{id}/mods/rescan` | `rescan_mods` | src/routes/mods.rs:163 | Path `id` | `read_dir`, `std::fs::read`, JAR parsing | +| GET | `/instances/{id}/backups` | `list_backups` | src/routes/backups.rs:30 | Path `id` | SQL SELECT | +| POST | `/instances/{id}/backups/create` | `create_backup` | src/routes/backups.rs:54 | Path `id` | `File::create`, `ZipWriter`, SQL INSERT | +| POST | `/backups/{id}/restore` | `restore_backup` | src/routes/backups.rs:88 | Path `id` | SQL SELECT (STUB — no-op) | +| GET | `/backups/{id}/download` | `download_backup` | src/routes/backups.rs:93 | Path `id` | SQL SELECT, `std::fs::read` | +| GET | `/accounts` | `manage_accounts` | src/routes/accounts.rs:60 | CurrentUser (must be superuser) | SQL SELECT | +| POST | `/accounts/create` | `create_account` | src/routes/accounts.rs:95 | `username`, `password`, `is_superuser` | SQL INSERT | +| POST | `/accounts/delete` | `delete_account` | src/routes/accounts.rs:111 | `form.id` | SQL DELETE | +| POST | `/accounts/set-password` | `set_password` | src/routes/accounts.rs:142 | `form.id`, `new_password` | SQL UPDATE, session DELETE | +| GET | `/change-password` | `change_password_page` | src/routes/mod.rs:58 | CurrentUser | None | +| POST | `/change-password` | `change_password` | src/routes/accounts.rs:163 | `current_password`, `new_password` | SQL UPDATE, session DELETE | + +--- + +## 3. Attacker-Controlled Input Sources (By Category) + +### 3a. Form Fields (application/x-www-form-urlencoded) +| Field | Handler(s) | Max Size | Validation | Risk | +|-------|-----------|----------|------------|------| +| `username` | authenticate, player actions, create_account, set_password | ~64KB | None (SQL param prevents injection) | LOW (SQL safe), **HIGH** (RCON injection in player actions) | +| `password` | authenticate, change_password, create_account | ~64KB | Argon2id hashed | LOW | +| `name` | create_instance | ~64KB | **NONE** | **CRITICAL** — used in fs::create_dir_all, stored in DB, used in remove_dir_all | +| `seed` | create_instance, regenerate_world | ~64KB | **NONE** — written to server.properties | **MEDIUM** — properties injection | +| `motd` | create_instance, edit_instance | ~64KB | **NONE** — passed to Docker env | **HIGH** — Docker env injection | +| `version` | create_instance | ~64KB | **NONE** — passed to Docker env | MEDIUM | +| `difficulty` / `gamemode` / `max_players` | create_instance, edit_instance | i32 | Range limited | MEDIUM | +| `memory_mb` | create_instance, edit_instance | i32 | 512..max_memory_mb | LOW | +| `server_type` | create_instance | ~64KB | CHECK constraint in SQL | LOW (DB enforced) | +| `level_type` | create_instance | ~64KB | Default "DEFAULT" | MEDIUM — Docker env | +| `pvp` | create_instance, edit_instance | ~8 chars | Checkbox (Some("on") | LOW | +| `world_source` | create_instance | ~64KB | CHECK constraint in SQL | LOW | + +### 3b. Multipart File Uploads +| Field | Handler | Max Size | Validation | Risk | +|-------|---------|----------|------------|------| +| File bytes (.jar) | upload_mod | 500MB per field | JAR metadata validation + class scan | **HIGH** — file write | +| File bytes (.zip) | upload_mod | 500MB per field | ZIP → extract .jar → JAR validation | **HIGH** — file write, nested ZIP | +| File bytes (.zip) | import_world | Multipart stream | `enclosed_name()` check | **HIGH** — world file write, zip slip edge cases | +| Filename (from Content-Disposition) | upload_mod | ~255 chars | `file_name()` strips dir components | **MEDIUM** — special chars preserved | +| Content-Type / boundary | upload_mod | ~255 chars | **NONE** — directly parsed for multer | **LOW** — multer handles internally | + +### 3c. Path Parameters +| Parameter | Handler(s) | Validation | Risk | +|-----------|-----------|------------|------| +| `id` (i64) | All /instances/{id}/*, /backups/{id}/* | Axum Path (integer only) | LOW — SQL uses params | +| `instance_id` (i64) | delete_mod | Axum Path<(i64, i64)> | LOW | +| `mod_id` (i64) | delete_mod | Axum Path<(i64, i64)> | LOW | + +### 3d. Cookie / Header Sources +| Source | Handler | What It Provides | Risk | +|--------|---------|-----------------|------| +| `Cookie: mcui_session` | require_auth (all protected routes) | Session ID → Account | **MEDIUM** — no server-side TTL | +| `Cookie: mcui_session` | logout | Session ID for deletion | **LOW** — missing auth guard | +| `Content-Type: boundary=` | upload_mod | Multipart parsing boundary | **LOW** | + +--- + +## 4. Sinks (Dangerous Operations) + +### 4a. Filesystem Write Sinks +| Sink | File:Line | Data Source | Traversal Protection | +|------|-----------|-------------|---------------------| +| `std::fs::create_dir_all(&instance_dir)` | src/routes/instances.rs:205 | `form.name` (user input) | **NONE** — `Path::join` traverses on `..` | +| `std::fs::write(&dest, bytes)` | src/routes/mods.rs:98 | `safe_name` (filename from upload) | Partial — `file_name()` strips dir | +| `std::fs::File::create(&out_path)` | src/routes/instances.rs:210 | `file.enclosed_name()` (ZIP entry path) | Partial — `enclosed_name()` checks | +| `std::fs::File::create(&backup_path)` | src/routes/backups.rs:72 | `filename` (timestamp-based) | LOW — controlled by app | +| `std::fs::write(&props_path, ...)` | src/routes/instances.rs:396 | `form.seed` (user input) | **NONE** — content injection | +| `std::fs::remove_dir_all(&instance_dir)` | src/routes/instances.rs:392-393 | `name` from DB (originally user input) | **NONE** | +| `std::fs::remove_dir_all(&world_dir)` | src/routes/instances.rs:421 | Instance name from DB | Partial | +| `std::fs::remove_file(&mod_path)` | src/routes/mods.rs:226 | `filename` from DB | Partial | + +### 4b. Filesystem Read Sinks +| Sink | File:Line | Data Source | Risk | +|------|-----------|-------------|------| +| `std::fs::read(&backup_path)` | src/routes/backups.rs:106 | `filename` from DB | **MEDIUM** — filename from DB could be traversed | +| `std::fs::read_dir(&data_dir)` | src/routes/backups.rs:68 | Instance name from DB | LOW | +| `std::fs::read_to_string(&props_path)` | src/routes/instances.rs:386 | Instance name from DB | LOW | +| `std::fs::read_dir(&mods_dir)` | src/routes/mods.rs:175 | Instance name from DB | LOW | +| `std::fs::read(&path)` | src/routes/mods.rs:183 | File path from read_dir | LOW | + +### 4c. RCON Command Sinks +| Sink | File:Line | Data Source | Command Template | Risk | +|------|-----------|-------------|-----------------|------| +| `client.send_command_async(cmd)` | src/routes/players.rs:97 | `form.username` | `format!("whitelist add {}", form.username)` | **HIGH** — command injection | +| `client.send_command_async(cmd)` | src/routes/players.rs:97 | `form.username` | `format!("op {}", form.username)` | **HIGH** | +| `client.send_command_async(cmd)` | src/routes/players.rs:97 | `form.username` | `format!("ban {}", form.username)` | **HIGH** | +| `client.send_command_async("seed".into())` | src/routes/instances.rs:153 | Hardcoded "seed" | `"seed"` | LOW | +| `client.send_command_async("list".into())` | src/routes/instances.rs:251 | Hardcoded "list" | `"list"` | LOW | + +### 4d. Docker Control Sinks +| Sink | File:Line | Data Source | Risk | +|------|-----------|-------------|------| +| `docker.start_container(&instance)` | src/routes/instances.rs:235 | Full Instance struct | **HIGH** — all settings go to Docker env | +| `docker.stop_container()` | src/routes/instances.rs:195 | None | LOW | +| `recreate_container()` | src/docker.rs:62 | `instance.name`, `instance.motd`, all settings | **HIGH** — env injection | +| `env_strings` construction | src/docker.rs:87-113 | `instance.version`, `instance.motd`, `instance.rcon_password`, etc. | **HIGH** — no escaping | + +### 4e. SQL Query Sinks +All SQL queries use `rusqlite::params![]` macro which provides parameterized queries. SQL injection is not feasible. + +### 4f. Template Rendering Sinks +| Sink | File:Line | Data | Risk | +|------|-----------|------|------| +| `template.render().unwrap_or_default()` | Various | User-controlled strings (name, motd, etc.) | **LOW** — Askama is compile-time template engine, no XSS injection possible | + +--- + +## 5. Hidden Control Channels + +| Channel | File:Line | Processing | What It Controls | Security Concern | +|---------|-----------|-----------|-----------------|-----------------| +| `Cookie: mcui_session` value | src/auth.rs:107 | Raw `split(';')` then `split_once('=')` | Identity (session ID) | Naive cookie parsing, no OWS stripping per RFC 6265. Cookie value with `=` or `;` could confuse parsing | +| `Content-Type: boundary=` | src/routes/mods.rs:141-148 | Custom parsing: `split(';')` → `strip_prefix("boundary=")` → `trim_matches('"')` | Multer multipart boundary | No validation on boundary value; multer validates internally but custom extraction may produce empty boundary | +| Instance properties → Docker env | src/docker.rs:87-113 | Direct `format!()` calls | Docker container environment | **No escaping** — newlines in MOTD or seed can inject arbitrary env vars | +| Instance name → filesystem path | src/routes/instances.rs:205 | `server_directory.join("instances").join(&form.name)` | Filesystem paths | **No sanitization** — `..` in name causes directory traversal | +| RCON password from DB → rendered HTML | src/routes/instances.rs:313 | `row.get(15)` (rcon_password) | Exposed to all authenticated users | Any authenticated user can see the RCON password for any instance | +| Seed → server.properties | src/routes/instances.rs:386-397 | Direct `format!("level-seed={seed}")` | server.properties content | **No escaping** — can inject arbitrary properties lines | +| Username → RCON command | src/routes/players.rs:93-97 | `format!("whitelist add {}", form.username)` | Minecraft server state | **No sanitization** — can inject arbitrary RCON commands | + +--- + +## 6. Exploit-Relevant Paths (Verified with Code Evidence) + +### X1: Instance Name Path Traversal → Arbitrary Directory Write +``` +POST /instances/new → CreateInstanceForm.name = "../../../tmp/evil" +→ src/routes/instances.rs:205: std::fs::create_dir_all(server_dir/instances/../../../tmp/evil/data) +→ SQL INSERT at line 119 with name stored as-is +→ Later: GET /instances/{id}/edit → edit_form reads name from DB +→ Later: POST /instances/{id}/delete → remove_dir_all(server_dir/instances/../../../tmp/evil) +``` +**Evidence:** `Path::join` on user-controlled `form.name` at line 173. No sanitization on name before `create_dir_all` at line 205. + +### X2: RCON Command Injection via Player Username +``` +POST /players/op/add → PlayerAction.username = "attacker; op injected_player" +→ src/routes/players.rs:93: format!("op {}", form.username) +→ src/routes/players.rs:97: client.send_command_async("op attacker; op injected_player") +``` +**Evidence:** `player_action!` macro at line 89 generates direct format string. No sanitization, no validation of username characters. + +### X3: MOTD Docker Environment Injection +``` +POST /instances/{id}/edit → EditInstanceForm.motd = "hello\nEULA=FALSE\nONLINE_MODE=true" +→ SQL UPDATE at line 186 (stored in DB) +→ POST /instances/{id}/activate → reads Instance struct +→ src/docker.rs:103: format!("MOTD={}", instance.motd) → env_strings.push("MOTD=hello\nEULA=FALSE\nONLINE_MODE=true") +→ Docker container env includes injected vars +``` +**Evidence:** `format!("MOTD={}", instance.motd)` at src/docker.rs:103. No escaping of newlines or special characters. + +### X4: Level-Seed Injection into server.properties +``` +POST /instances/{id}/world/regenerate → RegenerateWorldForm.seed = "123\nlevel-type=FLAT\ngamerule=doDaylightCycle=false\n" +→ src/routes/instances.rs:397: format!("level-seed={seed}") +→ Written to server.properties via line replacement (line 390-393) +→ Injected lines become new properties +``` +**Evidence:** Line 397: `new_lines.push(format!("level-seed={seed}"));` No escaping. Java `.properties` format treats newline-separated lines as separate entries. + +### X5: Mod Filename Path Traversal (Partially Mitigated) +``` +POST /instances/{id}/mods → multipart file with filename "../../../tmp/evil.jar" +→ src/routes/mods.rs:98: process_jar_upload → Path::new(filename).file_name() +→ "evil.jar" preserved, "../.." stripped by file_name() +→ Mitigation: streps directory components BUT unsafe write to mods_dir/evil.jar +``` +**Evidence:** `safe_name` derived from `file_name()` at line 96, then `mods_dir.join(&safe_name)` at line 98. + +### X6: No Server-Side Session TTL +``` +POST /login → sets cookie Max-Age=86400 (24h) +src/auth.rs:108-115: require_auth middleware executes: + "SELECT ... FROM sessions s JOIN accounts a ON s.account_id = a.id + WHERE s.id = ?1" + No "AND s.created_at > datetime('now', '-1 day')" filter +→ Sessions persist indefinitely in DB +``` +**Evidence:** Session query at src/auth.rs:113 has no time-based filter. Session ID from leaked cookie works forever. + +### X7: No CSRF Protection on Any POST Endpoint +``` +All POST handlers: No token validation, no origin/referer check, no SameSite=Strict +→ CSRF on: /logout, /instances/new, /instances/{id}/delete, /players/op/add, etc. +``` +**Evidence:** No CSRF middleware in src/routes/mod.rs. Cookie has SameSite=Lax (not Strict). All POST operations lack CSRF tokens. + +### X8: POST /logout Missing Auth Guard +``` +POST /logout registered on outer Router NOT behind require_auth +→ src/auth.rs:130-152: Reads Cookie header, extracts session ID, DELETEs from DB +→ No authentication required for the request +``` +**Evidence:** src/routes/mod.rs:20 registers logout on outer Router, before `.merge(protected)` at line 33. + +### X9: RCON Password Exposed to All Authenticated Users +``` +GET /instances/{id}/edit → edit_form handler +→ src/routes/instances.rs:313: rcon_password: row.get(15)? +→ Rendered in HTML template (EditInstanceTemplate) +→ Any authenticated user can see RCON password for any instance +``` +**Evidence:** SELECT query at line 301 includes `rcon_password`. No superuser or ownership check before rendering. + +### X10: Weak RCON Password Generation +``` +src/routes/instances.rs:523-529: generate_rcon_password() +→ rand::thread_rng() (not OsRng/crypto RNG) +→ 36^16 space (lowercase alphanumeric) +``` +**Evidence:** `rand::thread_rng()` at line 524 is not cryptographically secure. 62^16 would be stronger. + +### X11: Docker Env Variables Set Without Sanitization (Multiple) +ALL instance fields injected into Docker container env without sanitization: +- `VERSION={version}` (src/docker.rs:93) +- `MAX_PLAYERS={max_players}` (src/docker.rs:95) +- `DIFFICULTY={difficulty}` (src/docker.rs:96) +- `RCON_PASSWORD={rcon_password}` (src/docker.rs:109) +- `MOTD={motd}` (src/docker.rs:103) +- `LEVEL_TYPE={level_type}` (src/docker.rs:111) +- `LEVEL_SEED={seed}` (src/docker.rs:116) +- `GAMEMODE={gamemode}` (src/docker.rs:112) + +**Evidence:** src/docker.rs:87-116 — All `format!()` calls with direct `instance.*` values. No escaping. + +### X12: Backup Restore is No-Op (Data Loss Risk) +``` +POST /backups/{id}/restore → restore_backup handler +→ src/routes/backups.rs:88-96: Looks up backup in DB → redirects +→ NO actual file restoration performed +``` +**Evidence:** Handler at line 88 does SELECT then returns Redirect. No file copy/restore operations. + +### X13: Mod Validation DoS via Crafted JAR +``` +POST /instances/{id}/mods → upload crafted JAR with malicious constant pool +→ src/mods.rs:120: ZipArchive::new(data) +→ src/mods.rs:185: scan_single_class(data) — walks constant pool +→ No timeout, no recursion depth limit, no size limits on class files +→ Crafted constant pool (e.g., giant CONSTANT_Utf8 entries) could cause OOM +``` +**Evidence:** Loop at line 173 (`while i < cp_count`) has no iteration limit beyond cp_count. No bounds on CONSTANT_Utf8 length. + +### X14: Recursive ZIP Extraction Without Depth Limit +``` +POST /instances/{id}/mods → upload ZIP containing JAR (which is also a ZIP) +→ src/routes/mods.rs:131: process_zip_upload → extracts JAR bytes +→ src/routes/mods.rs:149: process_jar_upload → validate_mod +→ src/mods.rs:120: ZipArchive::new(data) — re-opens JAR as ZIP +→ No depth counter; possible infinite recursion +``` +**Evidence:** process_zip_upload calls process_jar_upload which calls validate_mod which opens the JAR as ZIP. No depth tracking. + +--- + +## 7. Trust Boundary Crossings + +| Boundary | Crossing Point | Direction | Data | Protection | +|----------|---------------|-----------|------|------------| +| HTTP → Auth | require_auth middleware (src/auth.rs:55) | Inbound | Session cookie → Account | Session lookup in DB | +| Auth → Instance Data | All protected handlers | Inbound | Authenticated request → CRUD | None (flat auth model) | +| WebUI → Filesystem | create_dir_all, fs::write, File::create | Outbound | User-controlled name/file content | Partial (mod filename) | +| WebUI → Docker | docker::start_container | Outbound | Instance settings | **NONE** | +| WebUI → RCON | RconClient::send_command_async | Outbound | Player username, commands | **NONE** | +| WebUI → Database | rusqlite execute/query | Outbound | Form data | Parameterized queries | +| WebUI → HTTP (Mojang) | reqwest::Client::get | Outbound | None (hardcoded URL) | Trusted URL | +| Upload → Validation | validate_mod (src/mods.rs:99) | Internal | Raw JAR bytes | Metadata + class scan | diff --git a/piolium/attack-surface/patch-bypass-summary.md b/piolium/attack-surface/patch-bypass-summary.md new file mode 100644 index 0000000..f7e7709 --- /dev/null +++ b/piolium/attack-surface/patch-bypass-summary.md @@ -0,0 +1,305 @@ +# Patch History & Bypass Summary + +**Audit ID:** `2026-07-28T23:03:56.306Z` +**Repository:** `/home/salmon/Documents/minecraft-ui` +**Branch:** `master` (HEAD `74d2ada`) +**Scan Window:** 500 commits / 60 days (all 6 commits captured) + +--- + +## Repository Overview + +Minecraft server management webUI built with **Rust/Axum** + **SQLite** + **Docker** (itzg/minecraft-server). +Manages multiple server instances, mods, backups, world files, whitelist/ops/bans. + +**Total commits in window:** 6 +**Security-dedicated commits:** **0** — no commits explicitly fix a disclosed vulnerability. +All commits are feature additions, cleanup, or initial scaffolding. + +--- + +## Commit Inventory & Security Relevance + +| # | Commit | Date | Type | Security Relevance | +|---|--------|------|------|--------------------| +| 1 | `0c2e661` | 2026-07-12 | Architecture | Setup only. No code. | +| 2 | `ae7ef10` | 2026-07-12 | First Stable | Introduced auth (Argon2 hashing, sessions), RCON, Docker, instance CRUD, backups. | +| 3 | `9ad52d0` | 2026-07-12 | Second Stable | Added world import/export/regenerate; `properties.rs` parser. **Introduced Zip upload surface.** | +| 4 | `ce30a87` | 2026-07-12 | Third Stable | Removed dead code; added RCON readiness poll; **added `file_name()` basename extraction** for path safety. | +| 5 | `97920e4` | 2026-07-12 | Config | Auto-create `config.toml` from default if missing. | +| 6 | `74d2ada` | 2026-07-29 | Mod Feature | Mod validation engine + upload; whitelist toggle; dynamic Java tags; **session cookie debug logging**; removed `hx-boost` (was breaking multipart). | + +**No advisory metadata exists for any commit.** Every change is a functional addition, not a security fix. + +--- + +## Security-Relevant Diffs (Per Commit) + +### Commit `ae7ef10` — Initial Authentication System + +**What was introduced:** +- Argon2 password hashing + verification +- Session-based auth with UUID v4 session IDs +- HttpOnly + SameSite=Lax cookie (max-age 86400s) +- Superuser guard on account management routes +- `require_auth` middleware applied to all protected routes + +**Potential bypass vectors evaluated:** + +| Vector | Assessment | +|--------|------------| +| Session prediction | UUID v4 is cryptographically random; no predictability. **Sound.** | +| Cookie theft via XSS | HttpOnly prevents JS access. But no `Secure` flag → cookie sent in cleartext over HTTP. **Bypassable** — network-level theft possible on LAN/public wifi. | +| Session fixation | New UUID generated on every login. No way to pre-set session ID. **Sound.** | +| SQL injection in session lookup | Uses parameterized queries. **Sound.** | +| Timing attack on password | Argon2 constant-time comparison. **Sound.** | +| Brute force | No rate limiting on `/login` endpoint. **Bypassable.** | +| Missing Secure flag | Cookie lacks `Secure`; SameSite=Lax helps but not on first request. **Bypassable.** | + +**Verdict: Relocated** — the auth primitive is sound in isolation, but the missing `Secure` flag and absent rate limiting shift the attack surface to network interception and brute force. + +--- + +### Commit `9ad52d0` — World Import (ZIP extraction) + +**What was introduced:** +- `import_world()` — multipart ZIP upload → extracted to `data/world/` +- `export_world()` — directory zipped and streamed to browser +- `regenerate_world()` — deletes world, rewrites `server.properties` + +**Security controls in import_world:** +- `file.enclosed_name()` from the `zip` crate prevents Zip Slip (path traversal) + +**Potential bypass vectors:** + +| Vector | Assessment | +|--------|------------| +| Zip Slip via symlink | `enclosed_name()` rejects entries whose path escapes the target dir. **Sound.** | +| Arbitrary file overwrite | Only files within `world_dir` can be written. **Sound.** | +| Zip bomb | No size limit on uploaded field. `bytes().await` loads entire upload into memory. **Bypassable** — OOM denial of service. | +| World seed manipulation | `regenerate_world()` writes user-supplied seed to `server.properties` without validation of seed content (e.g., newlines). Could inject extra properties. **Bypassable** — property injection. | + +**Specific bypass — property injection via seed:** +```rust +// In regenerate_world: +let seed = form.seed.unwrap_or_default(); +// ... +new_lines.push(format!("level-seed={seed}")); +``` +A seed containing newlines (`level-seed=123\nmax-players=999`) injects additional properties. + +**Verdict: Bypassable** — property injection via seed field; no upload size limits. + +--- + +### Commit `ce30a87` — Path Safety via `file_name()` + +**What was introduced:** +- Backup creation now uses `path.file_name()` to get basename before adding to ZIP +- Removed dead `container_status()` code +- RCON readiness polling (waits up to 60s for server to respond) + +**Security-relevant change:** +The switch to `file_name()` prevents path traversal in backup content (files inside `data/` are only stored by basename): +```rust +// Before (ae7ef10): +if let Some(name) = path.file_name() { ... } + +// After (ce30a87 — same pattern, already present) +``` + +This was already safe in the initial commit — no change. The commit just removed unused template fields and the `ContainerStatus` enum. + +**Verdict: Not a security fix.** No bypass applicable. + +--- + +### Commit `74d2ada` — Mod Validation Engine + +**What was introduced:** +- `src/mods.rs` — two-layer mod validation (metadata + class-file constant-pool scan) +- `src/routes/mods.rs` — CRUD routes for mod upload, rescan, delete +- Mods table in DB +- Whitelist enable/disable checkbox on edit page +- Dynamic Java image tag based on MC version +- Session cookie debug logging + +**Mod validation policy:** +- Only mods that **explicitly declare** themselves client-only in metadata are rejected +- Class-file scanning is informational only — never causes rejection +- Many Forge mods that work on servers reference `net/minecraft/client/...` (rendering/GUI) + +**Potential bypass vectors:** + +| Vector | Assessment | +|--------|------------| +| Malicious mod upload | No signature verification, no integrity check. Any server-capable mod with malicious code is accepted. **Bypassable by design** — the validation only catches honest client-only declarations. | +| Metadata spoofing | A server-side malicious mod declaring `side=CLIENT` in `mods.toml` would be **rejected** even though it works on servers. But a server-side malicious mod with `side=BOTH` or no metadata sails through. **Sound for the declared policy.** | +| Class scan evasion | Mod obfuscation that doesn't reference `net/minecraft/server/...` could bypass detection, but the scan is informational-only, so no security boundary to bypass. | +| Zip content extraction | `process_zip_upload()` extracts nested JARs from ZIPs. Uses `file_name()` basename. **Sound** — no directory traversal. | +| Path traversal in filename | `safe_name = Path::new(filename).file_name()` strips directory components. **Sound.** | +| Arbitrary file write via mod | Once accepted, mod JAR is written to `mods_dir`. No validation that file is actually a valid JAR. But Docker container mounts that dir, so any file ends up in `/data/mods/`. **Sound** — jar extension enforced by route, no symlink. | + +**Whitelist toggle fix:** +The `ENABLE_WHITELIST` env var was hardcoded `true` before this commit. Now it's configurable per-instance. This is a **feature addition** masquerading as a hardening — whitelist being off is a valid configuration choice, not a vulnerability. + +**Session cookie debug logging:** +Added tracing lines that log `raw_cookies` to stderr. If tracing output is captured in production logs, session IDs could leak into log files. **Potential information disclosure.** + +**Verdict: Sound for declared policy**, but the policy itself is weak — any malicious server-side mod is accepted. No bypass of the validation logic exists. + +--- + +## Cross-Cutting Vulnerability Analysis + +### 1. RCON Command Injection [UNFIXED] + +**Location:** `src/routes/players.rs` — `player_action!` macro + +**Mechanism:** +```rust +macro_rules! player_action { + ($name:ident, $command:expr) => { + async fn $name( + ... + Form(form): Form, + ) -> Result { + ... + let cmd = format!($command, form.username); + // e.g., "whitelist add {}", "op {}", "ban {}", etc. + client.send_command_async(cmd.clone()).await + } + }; +} +``` + +**Impact:** Username is directly interpolated into an RCON command string. An attacker can inject additional RCON commands via a crafted username: +``` +POST /players/whitelist/add +username=player1;say%20pwned +``` +Would execute: `whitelist add player1;say pwned` — both commands run. + +**Bypass:** No validation or sanitization of `form.username`. No character restrictions. RCON protocol sends commands as null-terminated strings; the `;` is interpreted by the Minecraft server's command parser, which chains commands. + +**Cluster:** This is a command injection class bug. No commit has attempted to fix it. + +--- + +### 2. Missing CSRF Protection [UNFIXED] + +**Location:** All form handlers + +**Mechanism:** No CSRF tokens on any POST form. `SameSite=Lax` on cookies provides partial protection (not for GET-based state changes, but all state changes use POST). However, `SameSite=Lax` sends cookies on top-level navigations from external sites, and `POST` forms from cross-origin without `SameSite=Strict` are still sent with cookies in some browser configurations. + +**Impact:** Cross-site request forgery on all admin actions (create instance, delete, activate, upload mods, manage players, change passwords, etc.). + +**Bypass:** No CSRF token validation exists anywhere. Attacker site can POST to any endpoint and the victim's browser will include the session cookie for top-level navigations. + +**Cluster:** No commit addresses CSRF. + +--- + +### 3. No Rate Limiting [UNFIXED] + +**Location:** `/login` endpoint, all POST handlers + +**Mechanism:** No rate limiting on any endpoint. Brute-force password attacks against the login form are unbounded. + +**Impact:** Unlimited login attempts for credential brute force. + +**Bypass:** Directly exploitable — no commit adds any throttling. + +--- + +### 4. Information Disclosure — World Seed via RCON [DESIGN ISSUE] + +**Location:** `src/routes/instances.rs` — `edit_form()` + +**Mechanism:** The world seed is fetched via RCON `seed` command and displayed on the edit page. Any authenticated user can see the seed. + +**Impact:** World seed is considered sensitive by some server operators (enables griefers to locate structures). The seed is also stored in DB and `server.properties`. + +**Not a patch bypass** — no commit has attempted to restrict seed access. + +--- + +### 5. Insecure Default Credentials [SOFT] + +**Location:** `config.toml` + +**Config:** +```toml +initial_superuser = "admin" +initial_superuser_password = "admin" +``` + +**Mechanism:** Default credentials are hardcoded in the example config. On first run, these create the initial superuser. `password_change_required=1` is set, forcing a password change on first login. + +**Bypass:** If an attacker gains access before the admin changes the password (e.g., upon deployment), they can log in with `admin:admin`. The forced password change can be bypassed if the attacker changes it to their own password. + +**Verdict: Relocated** — the forced-change mitigates but doesn't eliminate the window of exposure. + +--- + +### 6. Missing Authorization Boundaries [DESIGN ISSUE] + +**Location:** All instance routes + +**Mechanism:** All authenticated users can manage all instances. No per-instance access control. The `is_superuser` check only gates account management, not instance operations. + +**Impact:** Any user can create, delete, modify, activate, upload mods to, or back up any instance. Single tenancy by design, but problematic if user accounts are meant to isolate administrators. + +**Not a patch bypass** — no commit has attempted per-instance authorization. + +--- + +## Bypass Verdict Summary + +| # | Issue | Affected Since | Fix Status | Bypass Verdict | +|---|-------|---------------|------------|----------------| +| 1 | RCON command injection via username | `ae7ef10` | **UNFIXED** | Bypassable — no sanitization | +| 2 | Missing CSRF tokens | `ae7ef10` | **UNFIXED** | Bypassable — no protection | +| 3 | No login rate limiting | `ae7ef10` | **UNFIXED** | Bypassable — unbounded attempts | +| 4 | Cookie missing Secure flag | `ae7ef10` | **UNFIXED** | Bypassable — cleartext on HTTP | +| 5 | Property injection via world seed | `9ad52d0` | **UNFIXED** | Bypassable — no seed validation | +| 6 | Mod validation (policy weakness) | `74d2ada` | **BY DESIGN** | Sound — policy allows server mods | +| 7 | Zip bomb / no upload size limit | `9ad52d0` | **UNFIXED** | Bypassable — OOM DoS | +| 8 | Default admin credentials | `97920e4` | Relocated | Bypassable during initial window | +| 9 | Session log leakage | `74d2ada` | **UNFIXED** | Bypassable — if logs accessible | +| 10 | Static file serving without auth | `ae7ef10` | **BY DESIGN** | Sound — static dir is controlled | + +--- + +## Cluster IDs + +| Cluster ID | Commits | Theme | +|-----------|---------|-------| +| `C001` | `ae7ef10`, `9ad52d0`, `ce30a87`, `74d2ada` | Auth & Session Management | +| `C002` | `9ad52d0`, `ce30a87`, `74d2ada` | File Upload & Path Safety | +| `C003` | `9ad52d0`, `74d2ada` | Docker Container Configuration | +| `C004` | `74d2ada` | Mod Validation Engine | + +--- + +## Undisclosed Fix Candidates + +No commits in this repository appear to be silent (undisclosed) security fixes. All commits are feature additions or cleanup. The repository has no advisory history. + +However, the following patterns suggest latent vulnerabilities that **should** have been fixed but remain open: + +1. **RCON injection in `player_action!` macro** — the most severe unfixed issue (CWE-77) +2. **Property injection in `regenerate_world`** — seed field allows injecting arbitrary `server.properties` lines +3. **World import ZIP bomb** — no size limit on multipart field in `import_world()` + +--- + +## Conclusion + +- **0 security patches** found in the commit history. +- **3 critical unfixed vulnerabilities** (RCON injection, CSRF, rate limiting). +- **3 moderate unfixed issues** (cookie Secure flag, property injection, ZIP bomb). +- **2 design-level weaknesses** (mod validation policy, static file serving). +- No existing fix to bypass — the security controls that exist were built in from the initial commit and their bypassability is assessed above. + +The most actionable finding is **RCON command injection** (CWE-77) in the player management routes, which allows any authenticated user to execute arbitrary Minecraft server commands through crafted usernames. 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 diff --git a/piolium/attack-surface/sbom.json b/piolium/attack-surface/sbom.json new file mode 100644 index 0000000..3f7d7eb --- /dev/null +++ b/piolium/attack-surface/sbom.json @@ -0,0 +1,282 @@ +{ + "target": "salmon/minecraft-ui", + "generated_at": "2025-07-29T01:15:00Z", + "components": [ + { + "name": "rust", + "category": "runtime", + "ecosystem": null, + "version": "edition 2024", + "relationship": "direct", + "purpose": "Language runtime for the application", + "evidence": ["Cargo.toml:3 edition = \"2024\""], + "security_relevant": false + }, + { + "name": "axum", + "category": "framework", + "ecosystem": "crates.io", + "version": "0.8.9", + "relationship": "direct", + "purpose": "HTTP web framework (router, middleware, extractors)", + "evidence": ["Cargo.toml:7", "src/routes/mod.rs:3 use"], + "security_relevant": true + }, + { + "name": "multer", + "category": "package", + "ecosystem": "crates.io", + "version": "3.1.0", + "relationship": "direct", + "purpose": "Multipart/form-data parser for file uploads", + "evidence": ["Cargo.toml:8", "src/routes/mods.rs:106 use multer::Multipart"], + "security_relevant": true + }, + { + "name": "tokio", + "category": "package", + "ecosystem": "crates.io", + "version": "1.52.3", + "relationship": "direct", + "purpose": "Async runtime (TCP, timers, tasks)", + "evidence": ["Cargo.toml:9", "src/main.rs:15 #[tokio::main]"], + "security_relevant": true + }, + { + "name": "rusqlite", + "category": "package", + "ecosystem": "crates.io", + "version": "0.40.1", + "relationship": "direct", + "purpose": "SQLite database interface", + "evidence": ["Cargo.toml:11", "src/db.rs:2 use"], + "security_relevant": true + }, + { + "name": "bollard", + "category": "package", + "ecosystem": "crates.io", + "version": "0.17.1", + "relationship": "direct", + "purpose": "Docker Engine API client", + "evidence": ["Cargo.toml:12", "src/docker.rs:2-8 use bollard::*"], + "security_relevant": true + }, + { + "name": "argon2", + "category": "package", + "ecosystem": "crates.io", + "version": "0.5.3", + "relationship": "direct", + "purpose": "Password hashing (Argon2id algorithm)", + "evidence": ["Cargo.toml:17", "src/auth.rs:1-2 use argon2::*"], + "security_relevant": true + }, + { + "name": "zip", + "category": "package", + "ecosystem": "crates.io", + "version": "8.6.0", + "relationship": "direct", + "purpose": "ZIP archive reading/writing (world export/import, mods, backups)", + "evidence": ["Cargo.toml:20", "src/mods.rs:8 use zip::ZipArchive"], + "security_relevant": true + }, + { + "name": "reqwest", + "category": "package", + "ecosystem": "crates.io", + "version": "0.13.4", + "relationship": "direct", + "purpose": "HTTP client for Mojang version manifest API", + "evidence": ["Cargo.toml:26", "src/routes/instances.rs:280"], + "security_relevant": false + }, + { + "name": "cookie", + "category": "package", + "ecosystem": "crates.io", + "version": "0.18.1", + "relationship": "direct", + "purpose": "Cookie parsing utilities", + "evidence": ["Cargo.toml:28"], + "security_relevant": true + }, + { + "name": "axum-extra", + "category": "package", + "ecosystem": "crates.io", + "version": "0.12.6", + "relationship": "direct", + "purpose": "Extra axum utilities including cookie private jar", + "evidence": ["Cargo.toml:29", "features = [\"cookie-private\"]"], + "security_relevant": true + }, + { + "name": "serde", + "category": "package", + "ecosystem": "crates.io", + "version": "1.0.228", + "relationship": "direct", + "purpose": "Serialization/deserialization framework", + "evidence": ["Cargo.toml:13"], + "security_relevant": false + }, + { + "name": "askama", + "category": "framework", + "ecosystem": "crates.io", + "version": "0.16.0", + "relationship": "direct", + "purpose": "Compile-time template engine", + "evidence": ["Cargo.toml:10", "src/routes/*.rs: use askama::Template"], + "security_relevant": false + }, + { + "name": "tower-http", + "category": "package", + "ecosystem": "crates.io", + "version": "0.7.0", + "relationship": "direct", + "purpose": "HTTP middleware (static file serving via ServeDir)", + "evidence": ["Cargo.toml:15", "src/routes/mod.rs:6"], + "security_relevant": false + }, + { + "name": "chrono", + "category": "package", + "ecosystem": "crates.io", + "version": "0.4.45", + "relationship": "direct", + "purpose": "Date/time handling for backup timestamps", + "evidence": ["Cargo.toml:27", "src/routes/backups.rs:69"], + "security_relevant": false + }, + { + "name": "uuid", + "category": "package", + "ecosystem": "crates.io", + "version": "1.23.4", + "relationship": "direct", + "purpose": "UUID generation for session IDs", + "evidence": ["Cargo.toml:18", "src/auth.rs:82"], + "security_relevant": false + }, + { + "name": "tempfile", + "category": "package", + "ecosystem": "crates.io", + "version": "3.27.0", + "relationship": "direct", + "purpose": "Temporary file/directory management", + "evidence": ["Cargo.toml:21"], + "security_relevant": false + }, + { + "name": "rand", + "category": "package", + "ecosystem": "crates.io", + "version": "0.8.7", + "relationship": "direct", + "purpose": "Random number generation for RCON passwords", + "evidence": ["Cargo.toml:19", "src/routes/instances.rs:263"], + "security_relevant": true + }, + { + "name": "tracing", + "category": "package", + "ecosystem": "crates.io", + "version": "0.1.44", + "relationship": "direct", + "purpose": "Logging and instrumentation framework", + "evidence": ["Cargo.toml:22", "src/main.rs:3 use tracing_subscriber"], + "security_relevant": false + }, + { + "name": "serde_json", + "category": "package", + "ecosystem": "crates.io", + "version": "1.0.150", + "relationship": "direct", + "purpose": "JSON parsing (Fabric mod metadata, Mojang API)", + "evidence": ["Cargo.toml:14", "src/mods.rs: parse_fabric_metadata"], + "security_relevant": false + }, + { + "name": "toml", + "category": "package", + "ecosystem": "crates.io", + "version": "1.1.2+spec-1.1.0", + "relationship": "direct", + "purpose": "TOML config parsing + Forge mod metadata parsing", + "evidence": ["Cargo.toml:16", "src/config.rs:9", "src/mods.rs: parse_forge_metadata"], + "security_relevant": false + }, + { + "name": "futures-util", + "category": "package", + "ecosystem": "crates.io", + "version": "0.3.32", + "relationship": "direct", + "purpose": "Async stream utilities for Docker image pull", + "evidence": ["Cargo.toml:30", "src/docker.rs:6 use futures_util::StreamExt"], + "security_relevant": false + }, + { + "name": "Docker daemon", + "category": "binary", + "ecosystem": null, + "version": "unknown (local)", + "relationship": "direct", + "purpose": "Container runtime for Minecraft server instances", + "evidence": ["src/docker.rs:28 Docker::connect_with_local_defaults()"], + "security_relevant": true + }, + { + "name": "itzg/minecraft-server", + "category": "container-os", + "ecosystem": "docker", + "version": "java8 / java17 / java21 (dynamic per version)", + "relationship": "direct", + "purpose": "Minecraft server Docker image (Java + Minecraft server jar)", + "evidence": ["src/docker.rs:34-43 java_tag()", "minecraft/docker-compose.yml"], + "security_relevant": true + }, + { + "name": "piston-meta.mojang.com", + "category": "external-service", + "ecosystem": null, + "version": "N/A (API)", + "relationship": "direct", + "purpose": "Minecraft version manifest API", + "evidence": ["src/routes/instances.rs:280"], + "security_relevant": false + }, + { + "name": "htmx.min.js", + "category": "vendored", + "ecosystem": null, + "version": "unknown (vendored)", + "relationship": "direct", + "purpose": "HTMX JavaScript library for dynamic HTML", + "evidence": ["static/htmx.min.js"], + "security_relevant": false + }, + { + "name": "SQLite", + "category": "datastore", + "ecosystem": null, + "version": "bundled (via rusqlite bundled feature)", + "relationship": "direct", + "purpose": "Embedded database for accounts, sessions, instances, mods, backups", + "evidence": ["Cargo.toml:11 features = [\"bundled\"]", "src/db.rs"], + "security_relevant": true + } + ], + "categories_covered": ["runtime", "package", "framework", "datastore", "external-service", "container-os", "binary", "vendored"], + "coverage_gaps": [ + "build-ci: No CI configuration found in repository", + "container-os: Dockerfile not in repo — base image pulled externally (itzg/minecraft-server)", + "binary: Java runtime version is dynamic per Minecraft version, not pinned" + ] +} 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 diff --git a/piolium/attack-surface/spec-gap-summary.md b/piolium/attack-surface/spec-gap-summary.md new file mode 100644 index 0000000..5e52e81 --- /dev/null +++ b/piolium/attack-surface/spec-gap-summary.md @@ -0,0 +1,164 @@ +# P7 Spec, Framework Contract & Parser Gap Summary + +**Target:** salmon/minecraft-ui +**Date:** 2025-07-29 +**Phase:** P7 — Specification, Framework Contract & Parser Differential Analysis +**Mode:** deep + +--- + +## Overview + +This report documents gaps between authoritative specifications, framework contracts, and the codebase implementation for salmon/minecraft-ui. The analysis covers: + +- **RFC 9106** (Argon2) — password hashing parameter compliance +- **Java `.properties` format** (Oracle spec) — server.properties parsing +- **Source RCON Protocol** (Minecraft community spec — wiki.vg/RCON) — packet handling +- **RFC 6265** (HTTP State Management) — cookie parsing and session handling +- **Axum Framework Contracts** — middleware ordering, extension-based auth, multipart boundary parsing +- **Hidden Control Channels** — Docker environment injection, file path construction, host header trust +- **Deployment Assumptions** — reverse proxy, localhost binding, no TLS + +--- + +## Spec Gap Candidates + +### Gap 1: Java `.properties` — Missing Escape Sequence Processing + +- **Spec:** Oracle Java `.properties` file format +- **Requirement:** The format specifies escape sequences: `\n`, `\r`, `\t`, `\\`, `\uXXXX`, `\ ` (literal space), `\!`, `\#`, `\=`, `\:`, and multi-line continuation with trailing `\`. +- **Code Path:** `src/properties.rs:13-19` — `read_properties()` splits on `=` or `:` without escaping +- **Gap Type:** parsing | canonicalization +- **Impact:** Values containing `=`, `:`, or Java escape sequences in `server.properties` are misparsed. An attacker who can control `server.properties` values (e.g., via `level-seed` setting) can inject fake key-value pairs or corrupt parsing. The `level-seed` value in `server.properties` is written by `regenerate_world` via a line-replacement loop that creates `level-seed={seed}` — the seed value is not escaped for the `.properties` format. +- **Attack Vector:** Auth'd user sets a seed containing `\nlevel-type=FLAT\n` which, when written to `server.properties` via the naive line-replacement in `regenerate_world`, injects additional properties. On next server start, the injected properties alter server behavior. +- **Exploit Conditions:** Auth'd user; instance not active; access to `regenerate_world` handler. +- **Severity:** MEDIUM +- **Evidence:** `src/properties.rs:13-19` uses `split_once('=')` and `split_once(':')` with no escape processing. Java `.properties` spec requires backslash escaping for separators. + +### Gap 2: Argon2id (RFC 9106) — Non-Recommended Default Parameters + +- **RFC/Spec:** RFC 9106, Sections 3.1, 4, 7.4 +- **Requirement:** RFC 9106 §4 recommends as FIRST option: `Argon2id` with `t=1`, `p=4`, `m=2^21` (2 GiB). §7.4 states "Argon2id variant with t=1 and 2 GiB memory is the FIRST RECOMMENDED option". Salt SHOULD be 16 bytes and SHOULD be unique per password. +- **Code Path:** `src/auth.rs:28` — `Argon2::default().hash_password(password.as_bytes(), &salt)?` +- **Gap Type:** missing-check | parameter-choice +- **Impact:** `Argon2::default()` uses m_cost=19456 KiB (~19 MiB), t_cost=2 iterations, p_cost=1 lane. This is ~100x less memory than RFC 9106 recommends. While not a compliance vulnerability (any parameters are valid per the algorithm spec), the weak defaults reduce brute-force attack resistance relative to the RFC's recommended "default setting for all environments." +- **Attack Vector:** Offline password hash cracking benefits from the 19 MiB vs 2 GiB memory reduction (~100x cheaper per guess). +- **Exploit Conditions:** Attacker obtains password hash from database (SQLite file access or SQL injection). +- **Impact:** Accelerated offline brute-force by ~100x less memory per hash evaluation. +- **Severity:** MEDIUM +- **Evidence:** `src/auth.rs:28` uses `Argon2::default()` (m=19456 KiB, t=2, p=1). RFC 9106 §4 recommends m=2^21 KiB (2 GiB), t=1, p=4 for pass hashing. Source: `/home/salmon/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/argon2-0.5.3/src/params.rs:29` defines DEFAULT_M_COST = 19 * 1024. + +### Gap 3: RCON Protocol — Insufficient Response Read After EXECCOMMAND + +- **Spec:** Source RCON Protocol (Minecraft community convention — wiki.vg/RCON) +- **Requirement:** After sending `SERVERDATA_EXECCOMMAND` (type 2), the server responds with a `SERVERDATA_RESPONSE_VALUE` (type 0) packet containing the command output, followed by a second `SERVERDATA_RESPONSE_VALUE` (type 0) as an end marker. Multi-packet responses are possible. +- **Code Path:** `src/rcon.rs:72-76` — reads one response, then ignores errors on second +- **Gap Type:** state-machine | error-handling +- **Impact:** Reads only one guaranteed response. Fragmented multi-packet responses are truncated. The comment acknowledges this: "some servers send it, some don't; ignore errors." +- **Attack Vector:** Long RCON command output (> 1400 bytes) gets truncated. +- **Exploit Conditions:** Any RCON command with output exceeding ~1400 bytes. +- **Impact:** Truncated command response (information disclosure). +- **Severity:** LOW — protocol compliance only, no direct security bypass. +- **Evidence:** `src/rcon.rs:72` — `let response = self.read_packet(&mut stream)?;` followed by `let _ = self.read_packet(&mut stream);` with error ignored. + +### Gap 4: Java `.properties` — Key Continuation and Whitespace Separator Not Supported + +- **Spec:** Oracle Java `.properties` format +- **Code Path:** `src/properties.rs:13-19` +- **Gap Type:** parsing +- **Impact:** Lines with whitespace-only separators (e.g., `key value`) are silently ignored. +- **Severity:** LOW — informational; conventional Minecraft server.properties uses `=`. + +### Gap 5: Auth Middleware — Manual Cookie Parsing (RFC 6265) + +- **RFC/Spec:** RFC 6265 (HTTP State Management), Sections 4.2.2, 5 +- **Code Path:** `src/auth.rs:105-118` — manual `split(';')` then `split_once('=')` +- **Gap Type:** parsing | canonicalization +- **Impact:** The parser does NOT strip OWS around cookie names per RFC 6265 §4.2.2. Does NOT handle quoted cookie values. +- **Severity:** LOW — well-formed browser cookies not affected; no auth bypass possible. + +### Gap 6: Framework Contract — Logout Without Auth Middleware + +- **Contract:** Axum middleware; all state-changing ops require auth. +- **Code Path:** `src/routes/mod.rs:29` — `/logout` on outer router, not in `protected` group. +- **Gap Type:** middleware-ordering +- **Impact:** Anonymous requests can delete sessions (requires valid session ID — UUIDv4 impractical). +- **Severity:** LOW — theoretical; UUIDv4 unpredictability prevents exploitation. + +### Gap 7: Framework Contract — No Server-Side Session TTL Enforcement + +- **Contract:** Session cookie Max-Age=86400 but no server-side expiration check. +- **Code Path:** `src/auth.rs:93` — `Max-Age=86400`; `src/auth.rs:108-115` — no `created_at` check. +- **Gap Type:** missing-check +- **Impact:** Session rows remain valid indefinitely in DB. Stolen session ID (via XSS, log access, DB leak) works forever. +- **Attack Vector:** Session ID leak → permanent session hijacking. No server-side TTL means an attacker who obtains a session ID can reuse it indefinitely, long after the Max-Age expires. +- **Exploit Conditions:** Attacker must first obtain a valid session ID. +- **Impact:** Session hijacking window extended indefinitely. +- **Severity:** MEDIUM +- **Evidence:** `src/auth.rs:93` sets `Max-Age=86400` but `src/auth.rs:108-115` does `WHERE s.id = ?1` with no date filter. + +### Gap 8: Hidden Control Channel — Instance MOTD Injected Into Docker Env Without Sanitization + +- **Contract:** Docker env vars derived from instance settings; Docker env parsing splits on `\n`. +- **Code Path:** `src/docker.rs:103` — `format!("MOTD={}", instance.motd)` +- **Gap Type:** hidden-control-channel | missing-check +- **Impact:** MOTD containing newline characters (`%0a`) injects arbitrary additional env vars into the Docker container. For example, MOTD=`hello\nEULA=FALSE\nONLINE_MODE=true` would override security-critical env vars. All Docker env vars (VERSION, TYPE, MEMORY, MAX_PLAYERS, DIFFICULTY, RCON_PASSWORD, GAMEMODE, LEVEL_TYPE, LEVEL_SEED, PVP, MOTD) are set without sanitization. +- **Attack Vector:** Auth'd user edits instance MOTD → saves → triggers container restart → injected env vars alter Docker container behavior. +- **Exploit Conditions:** Auth'd user; container restarted after instance edit. +- **Impact:** Full Docker container environment variable injection — could override EULA, ONLINE_MODE, RCON settings, or inject arbitrary env vars. +- **Severity:** HIGH +- **Evidence:** `src/docker.rs:103` — `format!("MOTD={}", instance.motd)`. No escaping. MOTD sourced from `src/routes/instances.rs:206` via `Form` without sanitization. + +### Gap 9: Hidden Control Channel — Instance Name Used in File Paths Without Sanitization + +- **Contract:** Instance name → filesystem path. +- **Code Path:** `src/routes/instances.rs:173` — `server_directory.join("instances").join(&form.name)` +- **Gap Type:** hidden-control-channel | missing-check +- **Impact:** `std::path::Path::join` DOES traverse on `..` prefix. Instance name `../../evil` results in path outside intended tree. All subsequent operations (mod uploads, world import, backups) land at attacker-chosen locations. Files created: instance data directory, mod JAR files, world files. +- **Attack Vector:** Auth'd user creates instance with `../../evil_name` → data dir created at `server_directory/instances/../../evil_name/data` = `server_directory/evil_name/data`. Then upload mod or import world to write files at traversed location. +- **Exploit Conditions:** Auth'd user. +- **Impact:** Arbitrary file write relative to server_directory (WebUI process user permissions). +- **Severity:** HIGH +- **Evidence:** `src/routes/instances.rs:173` — `let instance_dir = state.config.server_directory.join("instances").join(&form.name).join("data");` No validation of `form.name` for `..` or path separators. Also stored in DB at `src/routes/instances.rs:119` without sanitization. + +### Gap 10: Hidden Control Channel — Content-Type Boundary Parsing Trusts Attacker Input + +- **Contract:** Multer multipart parser; boundary from Content-Type header. +- **Code Path:** `src/routes/mods.rs:140-149` — boundary parsed without validation +- **Gap Type:** hidden-control-channel | missing-check +- **Impact:** Attacker-controlled boundary value fed to `multer::Multipart::with_constraints`. While multer validates boundaries internally, a crafted boundary could cause field misidentification, size limit bypass, or field-skipping behavior. +- **Attack Vector:** Send crafted Content-Type header with boundary overlapping body content: malformed multipart parsing. +- **Exploit Conditions:** Auth'd user uploading mods. +- **Impact:** Potential DoS or file size limit bypass. +- **Severity:** MEDIUM +- **Evidence:** `src/routes/mods.rs:141-148` — boundary parsed from user-supplied `Content-Type`. + +### Gap 11: Hidden Control Channel — All Instance Settings In Docker Env Without Sanitization + +- **Contract:** All instance settings → Docker env. +- **Code Path:** `src/docker.rs:87-113` +- **Gap Type:** hidden-control-channel | missing-check +- **Impact:** Beyond MOTD (Gap 8), LEVEL_SEED, LEVEL_TYPE, GAMEMODE, DIFFICULTY, VERSION, RCON_PASSWORD are all injected without sanitization. +- **Severity:** HIGH — combined with Gap 8. + +### Gap 12: Framework Contract — RCON Password Exposed to All Authenticated Users + +- **Contract:** Supposed role-based access control for sensitive instance data. +- **Code Path:** `src/routes/instances.rs:313` — `rcon_password: row.get(15)?` in `edit_form` +- **Gap Type:** missing-check +- **Impact:** Any authenticated user can view any instance's RCON password by visiting `/instances/{id}/edit`. No superuser or instance-ownership check. RCON password grants full Minecraft server control. +- **Attack Vector:** Auth'd user navigates to `/instances/1/edit` → sees RCON password → uses RCON client to issue arbitrary commands. +- **Exploit Conditions:** Auth'd user. +- **Impact:** Full Minecraft server compromise via RCON — whitelist bypass, data destruction, command execution. +- **Severity:** HIGH +- **Evidence:** `src/routes/instances.rs:313` — `rcon_password` selected and returned. No superuser check. Compare with `src/routes/accounts.rs:35` where checks exist. + +--- + +## Cross-Reference with Phase 3 Domain Attack Research + +- **Gap 8/11** (Docker env injection) — NOT covered in Phase 3. New finding. +- **Gap 9** (Instance name path traversal) — NOT specifically covered. Phase 3 identified mod filename traversal (T2) but not instance name as traversal vector. +- **Gap 12** (RCON password exposure) — Covered in Phase 3 as T5. +- **Gap 7** (No session TTL) — Covered in Phase 3 domain research. +- **Gap 2** (Argon2 defaults) — NOT covered in Phase 3. diff --git a/piolium/attack-surface/state-concurrency-summary.md b/piolium/attack-surface/state-concurrency-summary.md new file mode 100644 index 0000000..48677b3 --- /dev/null +++ b/piolium/attack-surface/state-concurrency-summary.md @@ -0,0 +1,46 @@ +# State & Concurrency Audit + +## State-Holding Entities Catalogued + +| Entity | Table | State Columns | Allowable Values | Concurrency Risk | +|--------|-------|---------------|-----------------|------------------| +| Account | `accounts` | `is_superuser` | 0/1 | Low (rarely mutated) | +| Account | `accounts` | `password_change_required` | 0/1 | Low | +| Session | `sessions` | `created_at` | datetime | **High** — no TTL enforcement | +| Instance | `instances` | `is_active` | 0/1 | **CRITICAL** — background task stale writes | +| Instance | `instances` | `container_state` | stopped/pulling/booting/running/error | **CRITICAL** — not bound to instance ID in background writes | +| Instance | `instances` | `has_modpack` | 0/1 | Medium — RMW across mutex acquisitions | +| Instance | `instances` | `name` | TEXT (UNIQUE) | **HIGH** — name-reuse race between delete and create | +| Mod | `mods` | `accepted` | 0/1 | Low | +| Backup | `backups` | `filename`, `created_at` | TEXT, datetime | Medium — orphan ZIP on concurrent delete | +| All | — | — | — | **No idempotency infrastructure** | + +**Total state-holding entities catalogued:** 10 (across 5 tables) + +## Concurrency Primitives Observed + +| Primitive | Location | Scope | Limitation | +|-----------|----------|-------|------------| +| `Arc>` | `src/db.rs:7` | SQLite connection | Serialises per-statement, not per-transaction | +| `MutexGuard::drop()` implicit | Every `let conn = state.db.conn()` / `drop(conn)` pattern | Statement boundaries | Mutex released between read and write in multiple handlers | +| Tokio async tasks (background spawn) | `src/routes/instances.rs:457` | container lifecycle | **No instance ID binding** — uses `WHERE is_active = 1` | +| SQLite WAL mode | `src/db.rs:19` (PRAGMA) | Write-ahead logging | Allows concurrent reads during writes but does not prevent any of the races documented | + +**No distributed locks, no advisory locks, no `SELECT ... FOR UPDATE`, no explicit BEGIN/COMMIT boundaries.** + +## Idempotency Infrastructure + +**Absent.** No `idempotency_key` table, no deduplication tokens, no request ID storage, no unique constraints leveraged for dedup (except `accounts.username` and `instances.name`). Every POST handler is vulnerable to replay/double-submit. + +## Drafts Filed + +| Draft | Severity | Class | Summary | +|-------|----------|-------|---------| +| p6-001 | CRITICAL | toctou | Background task in `activate_instance` uses `WHERE is_active = 1` (no instance ID bound). After deactivation/reactivation, stale task writes `container_state` on the wrong instance. | +| p6-002 | HIGH | toctou | `delete_instance` releases DB mutex before `remove_dir_all`. Concurrent `create_instance` with same name has its data directory wiped. Cross-resource TOCTOU with data loss. | +| p6-003 | MEDIUM | state-machine-violation | `edit_instance` stops container on active instances without restarting. `is_active=1` and stale `container_state='running'` persist. Instance appears running but isn't. | +| p6-004 | HIGH | idempotency | No idempotency infrastructure on any POST handler. Mod upload, backup create, login, account create all vulnerable to replay/double-submit. | +| p6-005 | MEDIUM | rmw-no-txn | `process_jar_upload` writes file to disk before DB INSERT. Crash between file write and DB insert leaves orphaned mod file. Also performs `has_modpack` update across separate mutex acquisition. | +| p6-006 | MEDIUM | toctou | `create_backup` reads instance name, releases mutex, creates ZIP file (long I/O window), then re-acquires mutex. Concurrent instance deletion causes FK violation — orphaned ZIP file with no DB record. | + +**Total draft findings filed: 6** \ No newline at end of file diff --git a/piolium/attack-surface/unauthenticated-surface.md b/piolium/attack-surface/unauthenticated-surface.md new file mode 100644 index 0000000..d899854 --- /dev/null +++ b/piolium/attack-surface/unauthenticated-surface.md @@ -0,0 +1,110 @@ +# Unauthenticated Attack Surface + +Reachable by an anonymous attacker — no valid session, token, or API key. + +**Coverage**: 4 entry points | 3 by-design public | 1 missing-guard | 0 middleware-gap +**Auth model**: Cookie-based session (mcui_session UUIDv4) via `require_auth` middleware (src/auth.rs:55). Login sets Set-Cookie with HttpOnly; middleware extracts cookie header and queries sessions table. +**Coverage gaps**: None — all routes are statically registered in the axum Router at src/routes/mod.rs. No reflection-based or dynamically-registered handlers. + +## Pre-Auth HTTP / API Routes + +| # | Method | Path | Handler (file:line) | Why pre-auth | Notable inputs / sinks | Blast radius | +|---|--------|------|---------------------|--------------|------------------------|--------------| +| 1 | GET | `/login` | `login_page` (src/routes/mod.rs:53) | by-design | None — renders static template | None — no side effects, no data access | +| 2 | POST | `/login` | `authenticate` (src/auth.rs:34) | by-design | `username`, `password` form fields → Argon2id password hash verification → session INSERT | Account compromise if credentials brute-forced (no rate limiting — see p4-005). Session fixation not possible (new UUID on each login). | +| 3 | POST | `/logout` | `logout` (src/auth.rs:130) | **missing-guard** (p4-008) | Cookie header → raw string split on `;` and `=` → session ID → `DELETE FROM sessions WHERE id = ?1` | Session deletion if valid session ID known. UUIDv4 is unpredictable (122 bits). Low practical risk but defense-in-depth violation. | +| 4 | GET | `/static/*` | `ServeDir::new("static")` (src/routes/mod.rs:28) | by-design | File path after `/static/` → static file read | Static file read only. Directory contains only `style.css` and `htmx.min.js`. No user data, no secrets, no configuration. | + +## Other Unauthenticated Entry Points + +| Kind | Entry point (file:line) | Why pre-auth | Notes | +|------|-------------------------|--------------|-------| +| **Static file server** | `ServeDir::new("static")` mounted at Router level (src/routes/mod.rs:28) | by-design | Serves `static/` directory contents (style.css, htmx.min.js). No auth, no session check, no dynamic content. Served directly without any middleware. | +| **Logout session deletion** | `logout` handler (src/auth.rs:130) registered on **outer Router** (src/routes/mod.rs:20) | missing-guard (p4-008) | Covered by row #3 above. The handler is on the outer Router, NOT behind `require_auth`. Any anonymous request can reach it. The handler extracts session ID from Cookie header via raw string splitting (`split(';')` then `split_once('=')`) and attempts to delete the session from DB. | + +## Why `POST /logout` is classified `missing-guard` + +The route registration at src/routes/mod.rs:20: + +```rust +Router::new() + .route("/login", axum::routing::get(login_page)) + .route("/login", axum::routing::post(crate::auth::authenticate)) + .route("/logout", axum::routing::post(crate::auth::logout)) // <-- OUTSIDE protected group + .merge(protected) // <-- protected has .layer(auth_required) +``` + +The `require_auth` middleware (src/auth.rs:55) is applied only to the `protected` router group. The logout handler processes the request without any session validation — it simply reads the Cookie header, extracts the session ID by splitting on `;` and `=`, and executes `DELETE FROM sessions WHERE id = ?1`. + +**Mitigating factors:** +1. Session IDs are UUIDv4 (122 bits of entropy) — practically unpredictable +2. Attacker would need a victim's exact session cookie value +3. No account information is leaked; only session deletion occurs + +**Risk if session IDs leak:** +Session IDs could leak via: +- Server access logs (if session IDs appear in logged cookie headers) +- Error messages (currently suppressed by `unwrap_or` / `ok()` patterns) +- Database backup exposure +- Shoulder surfing / physical access + +If leaked, an attacker can force-logout the victim by sending `Cookie: mcui_session=` to `/logout`. + +## Pre-Auth Risk Summary + +The webUI is designed to bind to `127.0.0.1` and sit behind a reverse proxy (nginx), which mitigates direct anonymous access. If exposed to the internet (even unintentionally via misconfiguration, Docker port mapping, or reverse proxy error): + +| Risk | Pre-auth endpoint | Impact | Existing draft | +|------|-------------------|--------|----------------| +| **HIGH** | `POST /login` (no rate limit) | Account compromise via credential brute-force | p4-005 | +| **LOW** | `POST /logout` (missing guard) | Session deletion (requires valid session ID) | p4-008 | +| **HIGH** | All POST endpoints (no CSRF) | Cross-site request forgery on any state-changing operation | p4-003 | +| **NONE** | `GET /login` | None — static page | — | +| **NONE** | `GET /static/*` | None — only CSS/JS files | — | + +## Auth Bypass Verification + +**All protected routes are correctly behind `require_auth` middleware:** +- ✓ Dashboard (`/`) +- ✓ Instances CRUD (`/instances`, `/instances/{id}/edit`, etc.) +- ✓ World management (`/instances/{id}/world/*`) +- ✓ Player management (`/players`, `/players/*`) +- ✓ Backup management (`/instances/{id}/backups`, `/backups/{id}/*`) +- ✓ Mod management (`/instances/{id}/mods`, `/instances/{id}/mods/*`) +- ✓ Account management (`/accounts`, `/accounts/*`) +- ✓ Password change (`/change-password`) + +**Routes NOT behind `require_auth` (intentional for auth protocol):** +- `GET /login` — login form (by-design) +- `POST /login` — credential verification (by-design) +- `GET /static/*` — static assets (by-design) + +**Route NOT behind `require_auth` (anomaly):** +- `POST /logout` — session deletion (missing-guard, p4-008) + +## Hidden Control Channel Verification + +| Channel | Present? | Can it bypass auth? | +|---------|----------|---------------------| +| Cookie header (`mcui_session`) | Yes, in `require_auth` + `logout` | No — session must exist in DB | +| Content-Type boundary | Yes, in `upload_mod` handler | No — only affects file parsing | +| X-Forwarded-* headers | **Not read anywhere** | N/A (safe) | +| X-HTTP-Method-Override | **Not supported** | N/A (safe) | +| X-Admin / X-Internal / X-Debug | **Not supported** | N/A (safe) | +| Host header | **Not used for routing decisions** | N/A (safe) | + +## References to Existing Drafts + +The following p4 findings document pre-auth and authz issues in detail: + +| Draft | Class | Covers | +|-------|-------|--------| +| p4-003 | csrf | No CSRF on any POST endpoint | +| p4-005 | missing-rate-limit | No rate limiting on POST /login | +| p4-008 | missing-guard | POST /logout not behind auth middleware | +| p4-009 | cookie-no-secure-flag | Cookie missing Secure flag | +| p4-019 | cookie-parsing-naive | Naive cookie header string splitting | + +## Cross-Reference: Matrix Entry vs Unauthenticated Surface + +All entries in this file are derived from the authorization matrix (`piolium/attack-surface/public-routes-authz-matrix.md`). The matrix covers 37 total endpoints; 4 are reachable pre-auth as documented above. The remaining 33 require authentication via `require_auth` middleware. 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 @@ +# Variant Analysis Summary — Phase 12 + +**Generated:** 2025-07-29 +**Phase:** P12 (Variant Search) +**Status:** COMPLETE + +--- + +## Methodology + +For each of the 17 surviving Phase 10 findings, the entire codebase was searched for: +1. Same root cause pattern (unsanitized input → dangerous sink) +2. Same flow shape (source type → transformation → sink type) +3. Same attacker control level (authenticated/unauthenticated, direct/indirect) +4. Missing protections (no input validation, no escaping, no authorization check) + +--- + +## Results + +| Finding | Root Cause | Variants Found | New Drafts | +|---------|-----------|----------------|------------| +| 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 | +| p10-002 — RCON Command Injection | format!() with unsanitized username in RCON command | 0 | — | +| p10-003 — Level-Seed Properties Injection | format!() with unsanitized seed in Java .properties file | 0 | — | +| p10-004 — RCON Password Exposed | Sensitive field in template rendering | 0 | — | +| p10-005 — Missing CSRF Protection | Architectural (no CSRF tokens anywhere) | 0 (all 31 endpoints) | — | +| p10-006 — No Server-Side Session TTL | Single query point | 0 | — | +| p10-007 — No Rate Limiting Login | Architectural (no middleware) | 0 | — | +| p10-008 — Logout No Auth Middleware | Single route registration | 0 | — | +| p10-009 — Cookie No Secure Flag | Single cookie setting | 0 | — | +| p10-010 — Flat Authorization Model | Architectural (no per-resource authz) | 0 | — | +| p10-011 — Background Task Stale is_active | Background task closes over stale DB filter | 0 (all 5 updates in same task) | — | +| p10-012 — Delete-Create Name Reuse Race | TOCTOU race window | 0 | — | +| p10-013 — No Idempotency | Architectural (no idempotency keys) | 0 | — | +| p10-014 — Mod Write Before DB Orphan | Ordering bug (file before DB insert) | 0 | — | +| p10-015 — Backup Orphan Race | Ordering bug (ZIP before DB insert) | 0 | — | +| p10-016 — Mod Validation Denial of Service | No timeout on CPU-bound parsing | 0 | — | +| p10-017 — No Encryption at Rest | Architectural (no SQLite encryption) | 0 | — | + +--- + +## Path Traversal Variant Details + +### Variant 1: World Import Path Traversal (p12-001) + +| Property | Value | +|----------|-------| +| Source | `instances.name` from DB (originally `form.name`) | +| Sink | `std::fs::File::create()` at `instances.rs:704` | +| Flow | POST `/instances/{id}/world/import` → load name from DB → `.join(&instance_name)` → `world_dir.join(zip_entry)` → `File::create()` | +| Constraint | `/data/world/` suffix appended after traversed name | +| Severity | HIGH | +| New? | Yes — NOT in AP-001 registry previously | + +### Variant 2: Regenerate World Path Traversal (p12-002) + +| Property | Value | +|----------|-------| +| Source | `instances.name` from DB (originally `form.name`) | +| Sink | `std::fs::remove_dir_all()` at `instances.rs:779` | +| Flow | POST `/instances/{id}/world/regenerate` → load name from DB → `.join(&instance_name)` → `remove_dir_all(world_dir)` | +| Constraint | `/data/world/` suffix appended after traversed name | +| Severity | HIGH | +| New? | Yes — NOT in AP-001 registry previously | + +### Variant 3: Player Management Path Traversal (p12-003) + +| Property | Value | +|----------|-------| +| Source | `instances.name` from active instance query | +| Sink | `std::fs::read_to_string()` via `read_json_list()` at `players.rs:59-76` | +| Flow | GET `/players` → `get_active_instance()` → `.join(&inst_name)` → `read_json_list(.../whitelist.json)` | +| Constraint | Only files named `whitelist.json`, `ops.json`, `banned-players.json` are read | +| Severity | MEDIUM (read-only, constrained to 3 filenames) | +| New? | Yes — NOT in AP-001 registry previously | + +### Variant 4: Backup Create Path Traversal (p12-004) + +| Property | Value | +|----------|-------| +| Source | `instances.name` from DB (originally `form.name`) | +| Sink | `std::fs::File::create()` at `backups.rs:114` | +| Flow | POST `/instances/{id}/backups/create` → load name from DB → `filename = {name}_{timestamp}.zip` → `.join(&filename)` → `File::create()` | +| Constraint | Timestamp suffix (`_YYYYMMDD_HHMMSS.zip`) attached to traversed name | +| Severity | MEDIUM (cannot control exact filename) | +| New? | Partially — backup filename path was listed in registry but created via Write (not Read) | + +--- + +## No Variants Found — Rationale + +### AP-002 (RCON Injection) +The `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. + +### AP-003 (Properties Injection) +Only `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. + +### AP-004/005 (CSRF, Session TTL) +Architectural/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. + +### Concurrency (p10-011 through p10-015) +Each 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. + +### DoS (p10-016) +The 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. + +### Encryption (p10-017) +Single architectural gap — no encryption layer on SQLite. No variants. + +--- + +## Registry Updates + +The following new instances were added to AP-001 in the attack pattern registry: +- `src/routes/instances.rs:672 — world import via instance_name (FILE WRITE)` +- `src/routes/instances.rs:776 — regenerate world via instance_name (DIRECTORY DELETE)` +- `src/routes/players.rs:53 — player management via inst_name (FILE READ)` +- `src/routes/backups.rs:112 — backup create via filename (FILE WRITE)` + +--- + +## Conclusion + +**Total new variants found:** 4 +**All from:** AP-001 (Path Traversal via Unsanitized Instance Name) +**Total variant drafts written:** 4 (p12-001 through p12-004) +**Findings with zero variants:** 13 (p10-002 through p10-017, except p10-001) + +The 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). diff --git a/piolium/audit-state.json b/piolium/audit-state.json new file mode 100644 index 0000000..a3189ba --- /dev/null +++ b/piolium/audit-state.json @@ -0,0 +1,128 @@ +{ + "audits": [ + { + "audit_id": "2026-07-28T23:03:56.306Z", + "mode": "deep", + "started_at": "2026-07-28T23:03:56.306Z", + "completed_at": "2026-07-28T23:56:39.168Z", + "status": "complete", + "phases": { + "P1": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:03:56.339Z", + "completed_at": "2026-07-28T23:06:10.169Z" + }, + "P2": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:06:10.172Z", + "completed_at": "2026-07-28T23:08:39.158Z" + }, + "P3": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:08:39.163Z", + "completed_at": "2026-07-28T23:09:50.321Z" + }, + "P4": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:09:50.322Z", + "completed_at": "2026-07-28T23:15:54.779Z" + }, + "P5": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:15:54.785Z", + "completed_at": "2026-07-28T23:19:02.299Z" + }, + "P6": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-29T00:00:00.000Z", + "completed_at": "2026-07-28T23:33:30.924Z" + }, + "P7": { + "status": "complete", + "attempt": 2, + "max_attempts": 6, + "started_at": "2026-07-28T23:15:54.842Z", + "completed_at": "2026-07-28T23:18:53.050Z" + }, + "P8": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:33:30.925Z", + "completed_at": "2026-07-28T23:37:29.143Z" + }, + "P9": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:37:29.144Z", + "completed_at": "2026-07-28T23:37:43.969Z" + }, + "P10": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:37:43.971Z", + "completed_at": "2026-07-28T23:47:24.464Z" + }, + "P11": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:47:24.475Z", + "completed_at": "2026-07-28T23:53:33.336Z" + }, + "P12": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:53:33.338Z", + "completed_at": "2026-07-28T23:55:30.920Z" + }, + "P13": { + "status": "skipped", + "started_at": "2026-07-28T23:55:30.923Z", + "completed_at": "2026-07-28T23:55:30.931Z" + }, + "P14": { + "status": "skipped", + "started_at": "2026-07-28T23:55:30.931Z", + "completed_at": "2026-07-28T23:55:30.932Z" + }, + "P15": { + "status": "complete", + "attempt": 1, + "max_attempts": 6, + "started_at": "2026-07-28T23:55:30.933Z", + "completed_at": "2026-07-28T23:56:39.152Z" + }, + "P16": { + "status": "complete", + "started_at": "2026-07-28T23:56:39.153Z", + "completed_at": "2026-07-28T23:56:39.155Z" + }, + "P17": { + "status": "complete", + "started_at": "2026-07-28T23:56:39.155Z", + "completed_at": "2026-07-28T23:56:39.168Z" + } + }, + "agent_sdk": "pi", + "commit": "74d2ada067bd2fd9436710ab7ac260e0eddf1314", + "branch": "master", + "history_available": true + } + ] +} diff --git a/piolium/final-audit-report.md b/piolium/final-audit-report.md new file mode 100644 index 0000000..049d2b2 --- /dev/null +++ b/piolium/final-audit-report.md @@ -0,0 +1,407 @@ +# Security Audit Report: salmon/minecraft-ui + +========================================= + +## Executive Summary + +This audit of **minecraft-ui** — a self-hosted Rust (axum) web application that manages Docker-based Minecraft server instances — identified **17 validated vulnerabilities** spanning path traversal, injection, CSRF, session management, authorization, concurrency, denial-of-service, and cryptographic weaknesses. The most critical issue is **arbitrary filesystem access via unsanitized instance names** (AP-001), which enables path-traversal-driven directory creation, deletion, and constrained file read/write across 12 confirmed code paths — including 4 variant instances discovered in Phase 12. Four high-severity findings were cold-verified via adversarial review (P11). Combined with universal CSRF absence (31 POST endpoints) and a flat authorization model, the application provides minimal defense-in-depth: any authenticated user can destroy data, modify server configurations, or gain persistent access to Minecraft server consoles. The audit reached draft stage but the **findings promotion pipeline stalled** — no PoCs were built and no `piolium/findings/` directory was created. This report is assembled from available draft, chamber, and knowledge-base artifacts. + +## Methodology Summary + +- **Intelligence Gathering:** Advisory collection via `cargo-audit-advisory` (SBOM), architecture inventory of 16 Rust source files, dependency analysis (rusqlite, bollard, multer, zip) +- **Knowledge Base:** Threat modeling with DFD/CFD slices (P3), manual attack surface inventory of 38 routes (P8), domain attack research (Minecraft RCON, Java .properties, Docker env injection) +- **Static Analysis:** Structural extraction via manual code review; CodeQL database could not be built (system constraint); Semgrep ran but with limited Rust rule coverage +- **Review Chambers:** 6-cluster multi-agent debate system (path-traversal, injection, auth-session, state-concurrency, denial-of-service, security-architecture). 37 findings from P4/P5/P6/P8 reviewed → **17 VALID** survivors, 7 FALSE POSITIVE, 8 DROP, 5 DUPLICATE +- **Verification:** P11 cold verification (adversarial review) for 4 highest-severity findings: p10-001 (CONFIRMED), p10-003 (CONFIRMED), p10-004 (DISPROVED), p10-005 (CONFIRMED with severity downgrade challenge) +- **Variant Analysis (P12):** 4 path-traversal variants discovered from AP-001 root cause search + +## Summary of Findings + +| ID | Title | Severity | PoC Status | Status | +|----|-------|----------|------------|--------| +| — | Instance Name Path Traversal (p10-001) | CRITICAL | draft-only | Not promoted | +| — | Level-Seed Properties Injection (p10-003) | HIGH | draft-only | Not promoted | +| — | Missing CSRF Protection (p10-005) | HIGH | draft-only | Not promoted | +| — | RCON Password Exposed to All Users (p10-004) | **LOW** (DISPROVED) | blocked | Adversarial reversal | +| — | RCON Command Injection (p10-002) | MEDIUM | draft-only | Not promoted | +| — | Flat Authorization Model (p10-010) | MEDIUM | draft-only | Not promoted | +| — | No Server-Side Session TTL (p10-006) | MEDIUM | draft-only | Not promoted | +| — | No Rate Limiting Login (p10-007) | MEDIUM | draft-only | Not promoted | +| — | Background Task Stale is_active (p10-011) | MEDIUM | draft-only | Not promoted | +| — | Delete-Create Name Reuse Race (p10-012) | MEDIUM | draft-only | Not promoted | +| — | No Idempotency (p10-013) | MEDIUM | draft-only | Not promoted | +| — | Mod Validation Denial of Service (p10-016) | MEDIUM | draft-only | Not promoted | +| — | No Encryption at Rest (p10-017) | MEDIUM | draft-only | Not promoted | +| — | Logout No Auth Middleware (p10-008) | LOW | draft-only | Not promoted | +| — | Cookie No Secure Flag (p10-009) | LOW | draft-only | Not promoted | +| — | Mod Write Before DB Orphan (p10-014) | LOW | draft-only | Not promoted | +| — | Backup Orphan Race (p10-015) | LOW | draft-only | Not promoted | + +### Variant Findings (Phase 12) + +| Variant ID | Title | Severity | Parent | PoC Status | +|-----------|-------|----------|--------|------------| +| p12-001 | World Import Path Traversal | HIGH | p10-001 | draft-only | +| p12-002 | Regenerate World Path Traversal | HIGH | p10-001 | draft-only | +| p12-003 | Player Management Path Traversal | MEDIUM | p10-001 | draft-only | +| p12-004 | Backup Create Path Traversal | MEDIUM | p10-001 | draft-only | + +> **Note:** No findings were promoted to `piolium/findings/` with PoC scripts, evidence, or full report.md files. All entries above remain at draft stage in `piolium/findings-draft/`. + +--- + +## Technical Findings Detail + +### Instance Name Path Traversal (p10-001 — CRITICAL equivalent) + +- **Severity:** CRITICAL +- **Summary:** The instance `name` field flows unsanitized into `Path::join()` for 8 different filesystem operations. Rust's `Path::join()` follows `..` components, enabling arbitrary directory creation, deletion, and constrained file read/write outside the intended data directory. +- **Impact:** An authenticated attacker can: + - **Delete arbitrary directories** via `remove_dir_all` (path: `src/routes/instances.rs:388-392`) — e.g., name=`../../../target` deletes the project build directory + - **Create arbitrary directories** via `create_dir_all` (path: `src/routes/instances.rs:204-206`) + - **Write files to arbitrary locations** (constrained by `/mods/` or `/data/world/` suffix chaining via mod upload and world import) + - **Read files from arbitrary locations** (constrained by world export and backup download paths) +- **Root Cause:** `Path::join()` in Rust follows `..` components. User-controlled `form.name` is passed directly with no sanitization, length limits, or character restrictions. No validation exists at any of the 12 known join call sites. +- **Key Code Reference:** `src/routes/instances.rs:204-206` — `Path::join(&form.name)` → `create_dir_all` +- **PoC Status:** draft-only (no executable PoC built) +- **Detailed Report:** `piolium/findings-draft/p10-001-instance-name-path-traversal.md` +- **Adversarial Review:** `piolium/adversarial-reviews/p10-001-instance-name-path-traversal-review.md` — CONFIRMED, severity challenge rejected + +#### Attack Pattern: AP-001 + +This finding is the root of attack pattern **AP-001** (Path Traversal via Unsanitized Instance Name), which has 12 confirmed instances across 4 route modules: +- `src/routes/instances.rs:205` (create_dir_all) +- `src/routes/instances.rs:392` (remove_dir_all) +- `src/routes/mods.rs:126` (mods dir creation) +- `src/routes/mods.rs:343` (mod rescan) +- `src/routes/mods.rs:394` (mod delete) +- `src/routes/backups.rs:107` (backup path) +- `src/routes/instances.rs:593` (world export) +- `src/routes/instances.rs:739` (server.properties) +- `src/routes/instances.rs:672` (world import — variant) +- `src/routes/instances.rs:776` (regenerate world — variant) +- `src/routes/players.rs:53` (player management — variant) +- `src/routes/backups.rs:112` (backup create — variant) + +#### Variants + +| Variant ID | Title | Severity | Location | Flow | +|-----------|-------|----------|----------|------| +| p12-001 | World Import Path Traversal | HIGH | `instances.rs:672` | `instance_name` from DB → `world_dir.join(zip_entry)` → `File::create()` | +| p12-002 | Regenerate World Path Traversal | HIGH | `instances.rs:776` | `instance_name` from DB → `.join(&name)` → `remove_dir_all(world_dir)` | +| p12-003 | Player Management Path Traversal | MEDIUM | `players.rs:53` | `inst_name` → `.join(...)` → `read_json_list()` (read-only, 3 filenames) | +| p12-004 | Backup Create Path Traversal | MEDIUM | `backups.rs:112` | `name` → `{name}_{timestamp}.zip` → `.join(&filename)` → `File::create()` | + +Variant details: `piolium/findings-draft/p12-001-*.md` through `piolium/findings-draft/p12-004-*.md` + +--- + +### Level-Seed Properties Injection (p10-003 — HIGH) + +- **Severity:** HIGH +- **Summary:** The `POST /instances/{id}/world/regenerate` handler writes the user-supplied `seed` value into `server.properties` via line-based replacement with no escaping. Newline characters in the seed inject arbitrary Java `.properties` entries. +- **Impact:** Persistent modification of Minecraft server configuration: + - Set `online-mode=false` (disable authentication — any player can join as anyone) + - Set `enforce-whitelist=false` (bypass whitelist restrictions) + - Change difficulty, gamemode, max-players, PvP settings + - Inject malformed properties causing server startup failure (DoS) + - **Persistence:** Injected values survive across server restarts until manually cleaned +- **Root Cause:** `format!("level-seed={seed}")` writes the seed value directly into `server.properties` without escaping. The Java `.properties` spec treats newlines as property separators. The `properties.rs:13-19` reader also lacks escape-sequence processing (spec gap G1). +- **Key Code Reference:** `src/routes/instances.rs:715-759` — `regenerate_world` handler, line replacement loop +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-003-level-seed-properties-injection.md` +- **Adversarial Review:** `piolium/adversarial-reviews/p10-003-level-seed-properties-injection-review.md` — CONFIRMED, severity unchanged + +--- + +### Missing CSRF Protection (p10-005 — HIGH) + +- **Severity:** HIGH (adversarial challenge to MEDIUM rejected) +- **Summary:** The application has no CSRF protection across any of its 31 POST endpoints. No CSRF tokens are generated or validated. No Origin/Referer header validation exists. The session cookie uses `SameSite=Lax` which permits cookies on top-level cross-site POST navigations. +- **Impact:** Universal cross-site request forgery enabling: + - **Account takeover:** Cross-site password change (though `current_password` IS required for `/change-password`, the superuser `/accounts/set-password` does NOT require the current password) + - **Instance destruction:** Cross-site delete requests wipe server data + - **File system manipulation:** CSRF + path traversal (p10-001) to write files to arbitrary locations + - **Minecraft server compromise:** CSRF to op attackers on the Minecraft server via player management endpoints + - **Malicious mod upload:** Cross-site mod upload writes JAR files to server +- **Root Cause:** Cookie `SameSite=Lax` (not `Strict`) and zero CSRF token infrastructure. No axum CSRF middleware, no custom token generation, no header validation. +- **Key Code Reference:** `src/auth.rs:83-86` — Cookie with `SameSite=Lax`; entire `src/routes/mod.rs` — no CSRF middleware +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-005-missing-csrf-protection.md` +- **Adversarial Review:** `piolium/adversarial-reviews/p10-005-missing-csrf-protection-review.md` — CONFIRMED; severity down-challenge from HIGH to MEDIUM rejected by chamber (sub-claim D partially false but overall impact remains HIGH) + +--- + +### RCON Command Injection (p10-002 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** The `player_action!` macro in `src/routes/players.rs` constructs RCON commands by interpolating user-supplied usernames via `format!()` without sanitization. Six player management endpoints are affected. +- **Impact:** Operation-target manipulation via space injection, silent operation failure from malformed usernames. In vanilla Minecraft, RCON does not support command chaining (no `;` or `&&` injection), limiting impact to argument injection. On Spigot/Paper servers with command-chaining plugins, broader injection may be possible. +- **Root Cause:** `format!("whitelist add {}", form.username)` — no validation, no character filtering, no Minecraft username format enforcement. +- **Key Code Reference:** `src/routes/players.rs:89-102` — `player_action!` macro; `PlayerAction.username` is a raw `String` +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-002-rcon-command-injection.md` + +--- + +### Flat Authorization Model (p10-010 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** Any authenticated user has full access to ALL instances, mods, backups, and player operations. There is no per-resource ownership, no role granularity beyond binary superuser/non-superuser, and no instance-level authorization. +- **Impact:** In multi-user deployments, every user is a full administrator of every server instance. Data exfiltration, destructive operations, and Minecraft server control are all available to any authenticated account. +- **Root Cause:** No `owner_id` column on `instances` table; all handlers use `CurrentUser(_user)` with destructured-and-ignored user variable; all queries lack user-scoped filters. +- **Key Code References:** `src/routes/instances.rs` (all handlers), `src/routes/players.rs` (`player_action!`), `src/routes/backups.rs`, `src/routes/mods.rs` +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-010-flat-authorization-model.md` + +--- + +### No Server-Side Session TTL (p10-006 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** The `require_auth` middleware validates session cookies against the database but never checks the `created_at` timestamp, despite the column existing in the schema. Stolen session cookies remain valid indefinitely server-side. +- **Root Cause:** `WHERE s.id = ?1` query at `src/auth.rs:108-115` lacks a time-based filter. Cookie Max-Age=86400 is client-side only and trivially bypassed. +- **Key Code Reference:** `src/auth.rs:108-115` — session query without `created_at` filter +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-006-no-server-side-session-ttl.md` + +--- + +### No Rate Limiting Login (p10-007 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** `POST /login` has no rate limiting, account lockout, or progressive delay. Argon2id (~100ms/attempt) provides a weak mitigating factor. +- **Root Cause:** No rate-limiting middleware, no `governor` or equivalent crate in Cargo.toml. +- **Key Code Reference:** `src/auth.rs:39-92` — authenticate handler; `src/routes/mod.rs:44` — route registration +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-007-no-rate-limiting-login.md` + +--- + +### Background Task Stale is_active (p10-011 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** `activate_instance` spawns a background tokio task that updates `container_state` using `WHERE is_active = 1` instead of the concrete instance ID. If the user switches active instances during the 60-second polling window, the stale task writes to the wrong instance's state. +- **Root Cause:** 5 UPDATE statements at `src/routes/instances.rs:457-502` all use `WHERE is_active = 1` instead of `WHERE id = ?1`. +- **Key Code Reference:** `src/routes/instances.rs:457-502` — background tokio task +- **PoC Status:** draft-only (requires dynamic test) +- **Detailed Report:** `piolium/findings-draft/p10-011-background-task-stale-is-active.md` + +--- + +### Delete-Create Name Reuse Race (p10-012 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** `delete_instance` releases the DB mutex before `remove_dir_all`. A concurrent `create_instance` with the same name can create a new instance whose data directory is then wiped by the still-running deletion. +- **Root Cause:** Mutex released at `drop(conn)` between `DELETE FROM instances` and `remove_dir_all()` (microsecond-to-millisecond race window). +- **Key Code Reference:** `src/routes/instances.rs:360-396` +- **PoC Status:** draft-only (requires dynamic test) +- **Detailed Report:** `piolium/findings-draft/p10-012-delete-create-name-reuse-race.md` + +--- + +### No Idempotency (p10-013 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** Zero idempotency infrastructure across all 31 POST endpoints. Network request replay (HTMX retry, proxy retry, rapid clicks) causes duplicate operations. +- **Impact:** Duplicate mod uploads cause classloader conflicts on server restart (DoS). Duplicate backup creation wastes disk space. Duplicate player RCON commands are mostly harmless. +- **Root Cause:** No idempotency key table, no deduplication logic, no request ID storage. +- **Key Code Reference:** All POST endpoints in `src/routes/*.rs` +- **PoC Status:** draft-only (static hypothesis) +- **Detailed Report:** `piolium/findings-draft/p10-013-no-idempotency.md` + +--- + +### Mod Validation Denial of Service (p10-016 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** `validate_mod()` parses uploaded JAR constant pools with no timeout or resource limits. A crafted constant pool (cp_count=65535, expensive CONSTANT_Utf8 entries) causes CPU exhaustion on the async runtime. +- **Impact:** Async thread pool exhaustion; multiple concurrent crafted uploads make the server unresponsive. +- **Root Cause:** Unbounded `while i < cp_count` loop at `src/mods.rs:173-232` with no timeout wrapping. +- **Key Code Reference:** `src/mods.rs:173-232` — `scan_single_class()`; `src/routes/mods.rs:109-250` — no timeout around `validate_mod` +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-016-mod-validation-denial-of-service.md` + +--- + +### No Encryption at Rest (p10-017 — MEDIUM) + +- **Severity:** MEDIUM +- **Summary:** SQLite database `mcui.db` is stored unencrypted on disk with plaintext RCON passwords. Disclosure via prerequisite file read (p10-001 path traversal) or physical access exposes all secrets. +- **Root Cause:** `Connection::open(path)` at `src/db.rs:16` with no encryption (no SQLCipher, no individual field encryption). +- **Key Code Reference:** `src/db.rs:16` — `Connection::open(path)` +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-017-no-encryption-at-rest.md` + +--- + +### Logout No Auth Middleware (p10-008 — LOW) + +- **Severity:** LOW +- **Summary:** `POST /logout` is registered outside the `require_auth` protected router group. Any unauthenticated request can reach the logout handler. Requires prior session ID disclosure (UUIDv4, 122-bit entropy) for practical exploitation. +- **Key Code Reference:** `src/routes/mod.rs:20-33` — logout registered before `.merge(protected)` +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-008-logout-no-auth-middleware.md` + +--- + +### Cookie No Secure Flag (p10-009 — LOW) + +- **Severity:** LOW +- **Summary:** Session cookie lacks `Secure` flag. With localhost-bound traffic behind an HTTPS-terminating reverse proxy, the cookie travels unencrypted between proxy and app. +- **Key Code Reference:** `src/auth.rs:83-86` — cookie string missing `; Secure` +- **PoC Status:** draft-only +- **Detailed Report:** `piolium/findings-draft/p10-009-cookie-no-secure-flag.md` + +--- + +### Mod Write Before DB Orphan (p10-014 — LOW) + +- **Severity:** LOW +- **Summary:** `process_jar_upload` writes the mod file to disk before inserting the DB record. A crash between these operations orphans the file (invisible to UI, never cleaned up). +- **Key Code Reference:** `src/routes/mods.rs:234-260` +- **PoC Status:** draft-only (static hypothesis) +- **Detailed Report:** `piolium/findings-draft/p10-014-mod-write-before-db-orphan.md` + +--- + +### Backup Orphan Race (p10-015 — LOW) + +- **Severity:** LOW +- **Summary:** `create_backup` creates the ZIP file before inserting the DB record. Concurrent instance deletion during ZIP creation causes a silently-swallowed FK violation, leaving orphaned ZIP files on disk. +- **Key Code Reference:** `src/routes/backups.rs:85-139` +- **PoC Status:** draft-only (requires dynamic test) +- **Detailed Report:** `piolium/findings-draft/p10-015-backup-orphan-race.md` + +--- + +## Attack Surface Summary + +The complete attack surface inventory is documented at: + +| Artifact | Path | Description | +|----------|------|-------------| +| **Knowledge Base Report** | `piolium/attack-surface/knowledge-base-report.md` | Full architecture model, DFD/CFD slices, trust boundaries, component inventory (18 sections) | +| **Attack Pattern Registry** | `piolium/attack-pattern-registry.json` | 5 confirmed attack patterns (AP-001 through AP-005) with 12+32+1+31+1=77 total confirmed instances | +| **Manual Attack Surface Inventory** | `piolium/attack-surface/manual-attack-surface-inventory.md` | 38 HTTP routes catalogued with entry points, sinks, trust boundaries | +| **Source-Sink Flow Paths** | `piolium/attack-surface/source-sink-flows-all-severities.md` | All data flow paths from attacker input to dangerous sinks | +| **Unauthenticated Surface** | `piolium/attack-surface/unauthenticated-surface.md` | Routes accessible without authentication | +| **Spec Gap Summary** | `piolium/attack-surface/spec-gap-summary.md` | RFC/contract gaps (Java .properties, Argon2, RCON protocol, Axum contracts) | +| **Variant Analysis Summary** | `piolium/attack-surface/variant-summary.md` | Phase 12 variant search results | +| **State/Concurrency Summary** | `piolium/attack-surface/state-concurrency-summary.md` | Race conditions, TOCTOU, ordering bugs | +| **Advisory Summary** | `piolium/attack-surface/advisory-summary.md` | External advisory intelligence | +| **SBOM** | `piolium/attack-surface/sbom.json` | Software bill of materials | +| **Patch Bypass Summary** | `piolium/attack-surface/patch-bypass-summary.md` | Prior-sec patch regression analysis | + +### Key Attack Surface Dimensions + +1. **38 HTTP routes** — 3 public POST, 1 public GET, 31 authenticated POST, 3 authenticated GET +2. **5 attacker-controlled input sources** — form fields (instance name, seed, username, motd), multipart uploads (JAR/ZIP files), path parameters (instance ID, backup ID, mod ID), cookie header, outbound Mojang API response +3. **4 dangerous sinks** — filesystem (create_dir_all, remove_dir_all, File::create, read_to_string), RCON TCP (send_command_async), Docker Unix socket (bollard API), SQLite (rusqlite) +4. **3 trust boundaries** — unauthenticated→authenticated (session cookie), webUI→filesystem (path traversal), webUI→RCON/Minecraft (command injection) + +--- + +## Coverage Gaps + +### Pipeline Failure: Findings Never Promoted + +The most significant gap is that **the findings promotion pipeline terminated prematurely** between Phase 12 (variant analysis) and Phase 15 (report assembly): + +- `piolium/findings/` — **does not exist** — no finding was ever promoted from draft +- No PoC scripts were built for any finding +- No `report.md` files were generated with structured severity/PoC/evidence sections +- No execution evidence was collected +- Only 4 of 17 findings received adversarial review (P11 cold verification) +- No `metadata.json` files were created for variant-parent tracking +- Validating script (`validate_phase_output.py`) not present at expected path + +### Static Analysis Gaps + +- **CodeQL database could not be built** — the Rust analyzer version mismatch prevented database construction. Only structural extraction (call graphs, entry points, sinks) was performed from manual review. +- **Semgrep Pro** not available — cross-file taint analysis was not performed for Rust +- **No automated sink-to-source tracing** — all data flow mapping was manual + +### Multi-User Deployment Not Analyzed for Scale + +- The flat authorization model (p10-010) was assessed as MEDIUM based on single-user assumptions +- In a multi-user deployment (2+ users), the severity of p10-010 + p10-001 + p10-005 escalates to CRITICAL +- No instance-level access control, audit logging, or rate limiting for user actions + +### Encryption & Key Management + +- No encryption at rest (p10-017) +- RCON passwords stored in plaintext +- No TLS termination handled by the application (delegated to reverse proxy) +- Session IDs stored in unencrypted SQLite + +### Supply Chain Not Scanned + +- 19 Rust crate dependencies listed in SBOM but no automated vulnerability scan (cargo-audit not integrated) +- Outbound Mojang API calls not constrained (no TLS pinning, no timeout documented) + +--- + +## Methodology Notes + +### Chamber Workspace Summary + +The Phase 10 adversarial review chamber processed 37 finding candidates across 6 clusters: + +| Cluster | Attack Class | Candidates | Valid Survivors | Rejected | +|---------|-------------|-----------|-----------------|----------| +| path-traversal | Path Traversal & File System | 7 | 1 (H-01) | 6 | +| injection | Injection (RCON, Docker, Properties) | 5 | 2 (H-08, H-10) | 3 | +| auth-session | Auth & Session Management | 9 | 4 (H-13, H-14, H-16, H-17) | 5 | +| state-concurrency | State & Concurrency | 6 | 5 | 1 | +| denial-of-service | Denial of Service | 4 | 1 (H-28) | 3 | +| security-architecture | Security Architecture | 6 | 1 (H-33) | 5 | +| **Total** | | **37** | **17** | **20** | + +**Rejection breakdown:** 7 FALSE POSITIVE, 8 DROP, 5 DUPLICATE + +### Cold Verification (P11) + +4 findings subjected to adversarial review: +- **p10-001** (Instance Name Path Traversal): CONFIRMED — severity HIGH→HIGH (no change) +- **p10-003** (Level-Seed Properties Injection): CONFIRMED — severity HIGH→HIGH (no change) +- **p10-004** (RCON Password Exposed): DISPROVED — password never rendered in edit.html template (Askama explicit-field model) +- **p10-005** (Missing CSRF Protection): CONFIRMED — severity HIGH→MEDIUM challenged, rejected by chamber (CSRF+chain attacks warrant HIGH) + +### Variant Analysis (P12) + +4 new variant instances discovered from AP-001 root cause search: +- All 4 come from instance name path traversal pattern +- Zero variants found for the remaining 13 findings +- The path traversal pattern (AP-001) is the most pervasive vulnerability class with 12 total confirmed instances + +### Attack Pattern Registry + +5 attack patterns documented in `piolium/attack-pattern-registry.json`: +- AP-001: Path Traversal via Unsanitized Instance Name (12 instances) +- AP-002: RCON Command Injection via Unsanitized Username (6 instances) +- AP-003: Java Properties Injection via Unsanitized Seed Value (1 instance) +- AP-004: Missing CSRF Protection (31 endpoints) +- AP-005: No Server-Side Session TTL Enforcement (1 query) + +--- + +## Conclusion + +This audit catalogued 17 validated vulnerabilities across the minecraft-ui codebase. The most severe class — **path traversal via unsanitized instance names** — creates a systemic risk: any authenticated user can read, write, and delete files anywhere the webUI process can access. The absence of CSRF protection (31 endpoints) and per-resource authorization means that a single compromised account (or a cross-site request from a victim's browser) grants full control over all server instances, their files, and their Minecraft consoles. + +The application's security posture is appropriate _only_ for its documented single-user, localhost-only deployment model. Any deployment that: +- Exposes the WebUI beyond localhost +- Adds a second user account +- Sits behind a reverse proxy without CSRF middleware at the proxy layer +- Does not validate instance names at the input layer + +...faces unacceptable risk of data loss, privilege escalation, and persistent compromise. + +**Pipeline note:** The audit's findings-draft→findings promotion failed. The 17 validated findings, 4 variants, and 5 attack patterns documented in this report are derived from draft-level analysis without confirmed PoC execution. A follow-up audit should focus on building working PoCs for the critical and high-severity findings and executing them against a deployed instance to confirm exploitability and impact in a real environment. + +--- + +## Deferred Findings (triage skip) + +*No findings were triaged to `piolium/findings-deferred/`. The `piolium/findings-deferred/` directory does not exist because no findings were promoted past the draft stage. All 17 validated findings remain at investigative (draft) status.* \ No newline at end of file -- cgit v1.2.3