diff options
| author | salmonik <salmonik@tuta.io> | 2026-07-29 02:24:41 +0200 |
|---|---|---|
| committer | salmonik <salmonik@tuta.io> | 2026-07-29 02:24:41 +0200 |
| commit | c654a6d17eb58adbb15b9c47ab32bcc3a1666307 (patch) | |
| tree | 545544bf7d805254415da787fad1cdc711767833 /piolium/attack-surface/patch-bypass-summary.md | |
| parent | Add per-instance mod management with client-side detection (diff) | |
| download | minecraft-ui-c654a6d17eb58adbb15b9c47ab32bcc3a1666307.tar.gz minecraft-ui-c654a6d17eb58adbb15b9c47ab32bcc3a1666307.zip | |
Security audit fixes: patch 16 vulnerabilities across 7 source files
CRITICAL:
- Instance name path traversal (AP-001): sanitize::sanitize_instance_name()
strips .., /, \, \0, spaces, special chars at all 12 join call sites
- Zip-slip protection in world import: starts_with() guard after enclosed_name()
HIGH:
- RCON command injection: validate_username() rejects non-MC usernames
across all 6 player action macros (whitelist, op, ban)
- Level-seed properties injection: sanitize_seed() strips \n \r = : \0
- CSRF protection: SameSite=Lax -> SameSite=Strict on session cookie
MEDIUM:
- Session TTL: server-side 24h expiry via strftime check in require_auth
- Rate limiting: in-memory 10-ip/5min sliding window on login
- Background task stale is_active: all queries use concrete WHERE id=?1
- Mod validation DoS: 500MB cap, 10k class scan limit, 10k CP entry cap
LOW:
- Logout auth guard: moved into protected route group + CurrentUser param
- Mod orphan race: DB insert before file write
- Backup orphan race: DB insert before ZIP creation
- Template fix: broken Askama == comparisons in edit.html
All fixes compile clean with zero warnings. 10 unit tests pass.
Diffstat (limited to 'piolium/attack-surface/patch-bypass-summary.md')
| -rw-r--r-- | piolium/attack-surface/patch-bypass-summary.md | 305 |
1 files changed, 305 insertions, 0 deletions
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 @@ | |||
| 1 | # Patch History & Bypass Summary | ||
| 2 | |||
| 3 | **Audit ID:** `2026-07-28T23:03:56.306Z` | ||
| 4 | **Repository:** `/home/salmon/Documents/minecraft-ui` | ||
| 5 | **Branch:** `master` (HEAD `74d2ada`) | ||
| 6 | **Scan Window:** 500 commits / 60 days (all 6 commits captured) | ||
| 7 | |||
| 8 | --- | ||
| 9 | |||
| 10 | ## Repository Overview | ||
| 11 | |||
| 12 | Minecraft server management webUI built with **Rust/Axum** + **SQLite** + **Docker** (itzg/minecraft-server). | ||
| 13 | Manages multiple server instances, mods, backups, world files, whitelist/ops/bans. | ||
| 14 | |||
| 15 | **Total commits in window:** 6 | ||
| 16 | **Security-dedicated commits:** **0** — no commits explicitly fix a disclosed vulnerability. | ||
| 17 | All commits are feature additions, cleanup, or initial scaffolding. | ||
| 18 | |||
| 19 | --- | ||
| 20 | |||
| 21 | ## Commit Inventory & Security Relevance | ||
| 22 | |||
| 23 | | # | Commit | Date | Type | Security Relevance | | ||
| 24 | |---|--------|------|------|--------------------| | ||
| 25 | | 1 | `0c2e661` | 2026-07-12 | Architecture | Setup only. No code. | | ||
| 26 | | 2 | `ae7ef10` | 2026-07-12 | First Stable | Introduced auth (Argon2 hashing, sessions), RCON, Docker, instance CRUD, backups. | | ||
| 27 | | 3 | `9ad52d0` | 2026-07-12 | Second Stable | Added world import/export/regenerate; `properties.rs` parser. **Introduced Zip upload surface.** | | ||
| 28 | | 4 | `ce30a87` | 2026-07-12 | Third Stable | Removed dead code; added RCON readiness poll; **added `file_name()` basename extraction** for path safety. | | ||
| 29 | | 5 | `97920e4` | 2026-07-12 | Config | Auto-create `config.toml` from default if missing. | | ||
| 30 | | 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). | | ||
| 31 | |||
| 32 | **No advisory metadata exists for any commit.** Every change is a functional addition, not a security fix. | ||
| 33 | |||
| 34 | --- | ||
| 35 | |||
| 36 | ## Security-Relevant Diffs (Per Commit) | ||
| 37 | |||
| 38 | ### Commit `ae7ef10` — Initial Authentication System | ||
| 39 | |||
| 40 | **What was introduced:** | ||
| 41 | - Argon2 password hashing + verification | ||
| 42 | - Session-based auth with UUID v4 session IDs | ||
| 43 | - HttpOnly + SameSite=Lax cookie (max-age 86400s) | ||
| 44 | - Superuser guard on account management routes | ||
| 45 | - `require_auth` middleware applied to all protected routes | ||
| 46 | |||
| 47 | **Potential bypass vectors evaluated:** | ||
| 48 | |||
| 49 | | Vector | Assessment | | ||
| 50 | |--------|------------| | ||
| 51 | | Session prediction | UUID v4 is cryptographically random; no predictability. **Sound.** | | ||
| 52 | | 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. | | ||
| 53 | | Session fixation | New UUID generated on every login. No way to pre-set session ID. **Sound.** | | ||
| 54 | | SQL injection in session lookup | Uses parameterized queries. **Sound.** | | ||
| 55 | | Timing attack on password | Argon2 constant-time comparison. **Sound.** | | ||
| 56 | | Brute force | No rate limiting on `/login` endpoint. **Bypassable.** | | ||
| 57 | | Missing Secure flag | Cookie lacks `Secure`; SameSite=Lax helps but not on first request. **Bypassable.** | | ||
| 58 | |||
| 59 | **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. | ||
| 60 | |||
| 61 | --- | ||
| 62 | |||
| 63 | ### Commit `9ad52d0` — World Import (ZIP extraction) | ||
| 64 | |||
| 65 | **What was introduced:** | ||
| 66 | - `import_world()` — multipart ZIP upload → extracted to `data/world/` | ||
| 67 | - `export_world()` — directory zipped and streamed to browser | ||
| 68 | - `regenerate_world()` — deletes world, rewrites `server.properties` | ||
| 69 | |||
| 70 | **Security controls in import_world:** | ||
| 71 | - `file.enclosed_name()` from the `zip` crate prevents Zip Slip (path traversal) | ||
| 72 | |||
| 73 | **Potential bypass vectors:** | ||
| 74 | |||
| 75 | | Vector | Assessment | | ||
| 76 | |--------|------------| | ||
| 77 | | Zip Slip via symlink | `enclosed_name()` rejects entries whose path escapes the target dir. **Sound.** | | ||
| 78 | | Arbitrary file overwrite | Only files within `world_dir` can be written. **Sound.** | | ||
| 79 | | Zip bomb | No size limit on uploaded field. `bytes().await` loads entire upload into memory. **Bypassable** — OOM denial of service. | | ||
| 80 | | 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. | | ||
| 81 | |||
| 82 | **Specific bypass — property injection via seed:** | ||
| 83 | ```rust | ||
| 84 | // In regenerate_world: | ||
| 85 | let seed = form.seed.unwrap_or_default(); | ||
| 86 | // ... | ||
| 87 | new_lines.push(format!("level-seed={seed}")); | ||
| 88 | ``` | ||
| 89 | A seed containing newlines (`level-seed=123\nmax-players=999`) injects additional properties. | ||
| 90 | |||
| 91 | **Verdict: Bypassable** — property injection via seed field; no upload size limits. | ||
| 92 | |||
| 93 | --- | ||
| 94 | |||
| 95 | ### Commit `ce30a87` — Path Safety via `file_name()` | ||
| 96 | |||
| 97 | **What was introduced:** | ||
| 98 | - Backup creation now uses `path.file_name()` to get basename before adding to ZIP | ||
| 99 | - Removed dead `container_status()` code | ||
| 100 | - RCON readiness polling (waits up to 60s for server to respond) | ||
| 101 | |||
| 102 | **Security-relevant change:** | ||
| 103 | The switch to `file_name()` prevents path traversal in backup content (files inside `data/` are only stored by basename): | ||
| 104 | ```rust | ||
| 105 | // Before (ae7ef10): | ||
| 106 | if let Some(name) = path.file_name() { ... } | ||
| 107 | |||
| 108 | // After (ce30a87 — same pattern, already present) | ||
| 109 | ``` | ||
| 110 | |||
| 111 | This was already safe in the initial commit — no change. The commit just removed unused template fields and the `ContainerStatus` enum. | ||
| 112 | |||
| 113 | **Verdict: Not a security fix.** No bypass applicable. | ||
| 114 | |||
| 115 | --- | ||
| 116 | |||
| 117 | ### Commit `74d2ada` — Mod Validation Engine | ||
| 118 | |||
| 119 | **What was introduced:** | ||
| 120 | - `src/mods.rs` — two-layer mod validation (metadata + class-file constant-pool scan) | ||
| 121 | - `src/routes/mods.rs` — CRUD routes for mod upload, rescan, delete | ||
| 122 | - Mods table in DB | ||
| 123 | - Whitelist enable/disable checkbox on edit page | ||
| 124 | - Dynamic Java image tag based on MC version | ||
| 125 | - Session cookie debug logging | ||
| 126 | |||
| 127 | **Mod validation policy:** | ||
| 128 | - Only mods that **explicitly declare** themselves client-only in metadata are rejected | ||
| 129 | - Class-file scanning is informational only — never causes rejection | ||
| 130 | - Many Forge mods that work on servers reference `net/minecraft/client/...` (rendering/GUI) | ||
| 131 | |||
| 132 | **Potential bypass vectors:** | ||
| 133 | |||
| 134 | | Vector | Assessment | | ||
| 135 | |--------|------------| | ||
| 136 | | 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. | | ||
| 137 | | 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.** | | ||
| 138 | | 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. | | ||
| 139 | | Zip content extraction | `process_zip_upload()` extracts nested JARs from ZIPs. Uses `file_name()` basename. **Sound** — no directory traversal. | | ||
| 140 | | Path traversal in filename | `safe_name = Path::new(filename).file_name()` strips directory components. **Sound.** | | ||
| 141 | | 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. | | ||
| 142 | |||
| 143 | **Whitelist toggle fix:** | ||
| 144 | 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. | ||
| 145 | |||
| 146 | **Session cookie debug logging:** | ||
| 147 | 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.** | ||
| 148 | |||
| 149 | **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. | ||
| 150 | |||
| 151 | --- | ||
| 152 | |||
| 153 | ## Cross-Cutting Vulnerability Analysis | ||
| 154 | |||
| 155 | ### 1. RCON Command Injection [UNFIXED] | ||
| 156 | |||
| 157 | **Location:** `src/routes/players.rs` — `player_action!` macro | ||
| 158 | |||
| 159 | **Mechanism:** | ||
| 160 | ```rust | ||
| 161 | macro_rules! player_action { | ||
| 162 | ($name:ident, $command:expr) => { | ||
| 163 | async fn $name( | ||
| 164 | ... | ||
| 165 | Form(form): Form<PlayerAction>, | ||
| 166 | ) -> Result<Redirect, ...> { | ||
| 167 | ... | ||
| 168 | let cmd = format!($command, form.username); | ||
| 169 | // e.g., "whitelist add {}", "op {}", "ban {}", etc. | ||
| 170 | client.send_command_async(cmd.clone()).await | ||
| 171 | } | ||
| 172 | }; | ||
| 173 | } | ||
| 174 | ``` | ||
| 175 | |||
| 176 | **Impact:** Username is directly interpolated into an RCON command string. An attacker can inject additional RCON commands via a crafted username: | ||
| 177 | ``` | ||
| 178 | POST /players/whitelist/add | ||
| 179 | username=player1;say%20pwned | ||
| 180 | ``` | ||
| 181 | Would execute: `whitelist add player1;say pwned` — both commands run. | ||
| 182 | |||
| 183 | **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. | ||
| 184 | |||
| 185 | **Cluster:** This is a command injection class bug. No commit has attempted to fix it. | ||
| 186 | |||
| 187 | --- | ||
| 188 | |||
| 189 | ### 2. Missing CSRF Protection [UNFIXED] | ||
| 190 | |||
| 191 | **Location:** All form handlers | ||
| 192 | |||
| 193 | **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. | ||
| 194 | |||
| 195 | **Impact:** Cross-site request forgery on all admin actions (create instance, delete, activate, upload mods, manage players, change passwords, etc.). | ||
| 196 | |||
| 197 | **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. | ||
| 198 | |||
| 199 | **Cluster:** No commit addresses CSRF. | ||
| 200 | |||
| 201 | --- | ||
| 202 | |||
| 203 | ### 3. No Rate Limiting [UNFIXED] | ||
| 204 | |||
| 205 | **Location:** `/login` endpoint, all POST handlers | ||
| 206 | |||
| 207 | **Mechanism:** No rate limiting on any endpoint. Brute-force password attacks against the login form are unbounded. | ||
| 208 | |||
| 209 | **Impact:** Unlimited login attempts for credential brute force. | ||
| 210 | |||
| 211 | **Bypass:** Directly exploitable — no commit adds any throttling. | ||
| 212 | |||
| 213 | --- | ||
| 214 | |||
| 215 | ### 4. Information Disclosure — World Seed via RCON [DESIGN ISSUE] | ||
| 216 | |||
| 217 | **Location:** `src/routes/instances.rs` — `edit_form()` | ||
| 218 | |||
| 219 | **Mechanism:** The world seed is fetched via RCON `seed` command and displayed on the edit page. Any authenticated user can see the seed. | ||
| 220 | |||
| 221 | **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`. | ||
| 222 | |||
| 223 | **Not a patch bypass** — no commit has attempted to restrict seed access. | ||
| 224 | |||
| 225 | --- | ||
| 226 | |||
| 227 | ### 5. Insecure Default Credentials [SOFT] | ||
| 228 | |||
| 229 | **Location:** `config.toml` | ||
| 230 | |||
| 231 | **Config:** | ||
| 232 | ```toml | ||
| 233 | initial_superuser = "admin" | ||
| 234 | initial_superuser_password = "admin" | ||
| 235 | ``` | ||
| 236 | |||
| 237 | **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. | ||
| 238 | |||
| 239 | **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. | ||
| 240 | |||
| 241 | **Verdict: Relocated** — the forced-change mitigates but doesn't eliminate the window of exposure. | ||
| 242 | |||
| 243 | --- | ||
| 244 | |||
| 245 | ### 6. Missing Authorization Boundaries [DESIGN ISSUE] | ||
| 246 | |||
| 247 | **Location:** All instance routes | ||
| 248 | |||
| 249 | **Mechanism:** All authenticated users can manage all instances. No per-instance access control. The `is_superuser` check only gates account management, not instance operations. | ||
| 250 | |||
| 251 | **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. | ||
| 252 | |||
| 253 | **Not a patch bypass** — no commit has attempted per-instance authorization. | ||
| 254 | |||
| 255 | --- | ||
| 256 | |||
| 257 | ## Bypass Verdict Summary | ||
| 258 | |||
| 259 | | # | Issue | Affected Since | Fix Status | Bypass Verdict | | ||
| 260 | |---|-------|---------------|------------|----------------| | ||
| 261 | | 1 | RCON command injection via username | `ae7ef10` | **UNFIXED** | Bypassable — no sanitization | | ||
| 262 | | 2 | Missing CSRF tokens | `ae7ef10` | **UNFIXED** | Bypassable — no protection | | ||
| 263 | | 3 | No login rate limiting | `ae7ef10` | **UNFIXED** | Bypassable — unbounded attempts | | ||
| 264 | | 4 | Cookie missing Secure flag | `ae7ef10` | **UNFIXED** | Bypassable — cleartext on HTTP | | ||
| 265 | | 5 | Property injection via world seed | `9ad52d0` | **UNFIXED** | Bypassable — no seed validation | | ||
| 266 | | 6 | Mod validation (policy weakness) | `74d2ada` | **BY DESIGN** | Sound — policy allows server mods | | ||
| 267 | | 7 | Zip bomb / no upload size limit | `9ad52d0` | **UNFIXED** | Bypassable — OOM DoS | | ||
| 268 | | 8 | Default admin credentials | `97920e4` | Relocated | Bypassable during initial window | | ||
| 269 | | 9 | Session log leakage | `74d2ada` | **UNFIXED** | Bypassable — if logs accessible | | ||
| 270 | | 10 | Static file serving without auth | `ae7ef10` | **BY DESIGN** | Sound — static dir is controlled | | ||
| 271 | |||
| 272 | --- | ||
| 273 | |||
| 274 | ## Cluster IDs | ||
| 275 | |||
| 276 | | Cluster ID | Commits | Theme | | ||
| 277 | |-----------|---------|-------| | ||
| 278 | | `C001` | `ae7ef10`, `9ad52d0`, `ce30a87`, `74d2ada` | Auth & Session Management | | ||
| 279 | | `C002` | `9ad52d0`, `ce30a87`, `74d2ada` | File Upload & Path Safety | | ||
| 280 | | `C003` | `9ad52d0`, `74d2ada` | Docker Container Configuration | | ||
| 281 | | `C004` | `74d2ada` | Mod Validation Engine | | ||
| 282 | |||
| 283 | --- | ||
| 284 | |||
| 285 | ## Undisclosed Fix Candidates | ||
| 286 | |||
| 287 | 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. | ||
| 288 | |||
| 289 | However, the following patterns suggest latent vulnerabilities that **should** have been fixed but remain open: | ||
| 290 | |||
| 291 | 1. **RCON injection in `player_action!` macro** — the most severe unfixed issue (CWE-77) | ||
| 292 | 2. **Property injection in `regenerate_world`** — seed field allows injecting arbitrary `server.properties` lines | ||
| 293 | 3. **World import ZIP bomb** — no size limit on multipart field in `import_world()` | ||
| 294 | |||
| 295 | --- | ||
| 296 | |||
| 297 | ## Conclusion | ||
| 298 | |||
| 299 | - **0 security patches** found in the commit history. | ||
| 300 | - **3 critical unfixed vulnerabilities** (RCON injection, CSRF, rate limiting). | ||
| 301 | - **3 moderate unfixed issues** (cookie Secure flag, property injection, ZIP bomb). | ||
| 302 | - **2 design-level weaknesses** (mod validation policy, static file serving). | ||
| 303 | - No existing fix to bypass — the security controls that exist were built in from the initial commit and their bypassability is assessed above. | ||
| 304 | |||
| 305 | 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. | ||
