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/knowledge-base-report.md | 597 ++++++++++++++++++++++++ 1 file changed, 597 insertions(+) create mode 100644 piolium/attack-surface/knowledge-base-report.md (limited to 'piolium/attack-surface/knowledge-base-report.md') 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. -- cgit v1.2.3