Adversarial input-validation audit — every dangerous sink fails closed

Type: negative-path / input-validation audit (no production code change)
Date: 2026-07-08
Area: internal/api, internal/naming, internal/submit, internal/rcon
Task: a different question from the #82 / round-2 mutation audits. Those asked do the TESTS catch a gate regression? This asks does the CODE reject hostile INPUT, or does bad data PASS? — feed the real endpoints malformed, boundary, and hostile bodies (故意加错误数据) and confirm they fail closed (4xx) rather than letting the garbage reach a sink.

Method — sink-first, not fuzz-everything

The low-hanging garbage (oversized body, unknown field, wrong content-type) is already caught by the universal body guards, so a blanket "fuzz all ~80 handlers" would burn effort where the answer is known. The real "can bad data PASS?" risk lives at the sinks — the few places a request string is concatenated into an RCON command, used as a K8s object name, joined into a filesystem/archive path, put in a SQL query, or accepted as an enum/quantity without a validator in front. So the audit traces each dangerous sink class from its handler entry to the sink, and for the crown-jewel class (text → RCON) mutation-verifies the guard is non-vacuously pinned: loosen the guard in source, run the package tests, confirm the specifically-named negative test reddens (--- FAIL: <subtest>), then revert. Oracle: WSL Fedora-44, go1.26.4.

The universal belt (caught before any sink)

decodeJSON (internal/api/util.go) wraps every body in http.MaxBytesReader(w, r.Body, 1<<20) (1 MiB cap), sets DisallowUnknownFields(), and rejects trailing data after the first JSON value — all → 400 bad_request. requireJSONContentType returns 415 unsupported_media_type on credential writes (a CSRF belt). So oversized, unknown-field, multi-document, and wrong-type bodies never reach a handler body at all.

The dangerous sinks — each traced fail-closed

1. Text → RCON (the lead). Two vectors, both fenced.

(a) Structured commands — handlers_access.go (LuckPerms permission/group, whitelist/ban/kick). Every operand is validated against an anchored allow-list charset before it is concatenated: mcNameRe = ^[A-Za-z0-9_]{1,16}$ (player), lpNodeRe = ^[A-Za-z0-9_.*-]{1,64}$ (node), lpCtxRe = ^[A-Za-z0-9_-]{1,48}$ (world/group). No space, separator, or control character can appear in a validated operand, and there is no free-text field anywhere — a ban/kick deliberately carries no reason string (that would be the one splice vector). Go's $ is \z (absolute end, not \Z), so even a single trailing \n is rejected. Mutation-verified: loosening mcNameRe to admit a space (^[A-Za-z0-9_ ]{1,16}$) reddens TestAccessInjectionRejected/{whitelist,ban,kick,permission}_player_space — the guard is real, not vacuous.

(b) Free-text passthrough — handlers_console.go handleCommand, POST /servers/{name}/command. This is the one deliberate free-text → RCON vector, and it is owner/admin-gated (403 for a stranger, 404 for an unknown server). Its input fence: trim + strip a single leading /, reject empty, cap at 1024 bytes, and strings.IndexFunc(command, func(c rune) bool { return c < 0x20 }) >= 0 → 400 — every C0 control (incl. \n) is rejected so one request cannot splice a second command. Mutation-verified: disabling the scan (c < 0x20 → c < 0x00) reddens TestConsoleCommand/control_character_(newline)_->_400,_no_RCON_call, whose input is literally {"command":"say hi\nop attacker"} and whose assertion is 400 and console.calls == 0. (Severity note: because this vector is owner-gated by design, the scan is an audit-integrity measure — one request = one command — not a privilege boundary; a splice on your own server escalates nothing, since the owner may already run any RCON command. The fence exists regardless.)

2. Break-glass / internal-face (the newest code — scrutinised specifically)

This surface runs under a "service-token-authed / local-root, inputs trusted" posture, the classic place a field-level guard gets skipped. Traced end to end:

Server name — every internal handler (handleReady, handleJoinEvent, handleInternalWake, handleInternalClaim, handleInternalMenuStatus) validates the path name with naming.ValidateServerName before use.
mc_uuid (join/wake/claim bodies) — checked non-empty, then flows only to DB-parameterized calls (RecordJoin, UserByMCUUID, UUIDInAllowlist). The "allowlist" is a DB table, not a live RCON whitelist add — there is no mc_uuid → RCON path.
Break-glass "OP-create" (performAddOperator → provisionOperator → InsertOperator(ctx, id, username, email)) is a parameterized DB INSERT creating a panel staff account, not a Minecraft op RCON command. The hypothesised name → RCON op sink was checked and does not exist in this shape; the username is TrimSpaced and reaches only $N-parameterized SQL, from a local-root caller.
os_user (internal backup attribution) — TrimSpaced, sets only the audit actor (a DB row); shown non-vacuous in the round-2 backup/restore audit (b7b4a3b).

3. SQL injection — dismissed.

pgrepo.go uses uniform $1/$2/$3 parameterization throughout (QueryRowContext/ExecContext(ctx, q, args…)); no request string is Sprintf'd into a query.

4. Path traversal (submit) — dismissed.

internal/submit validates the submission id (rejects .., path separators, uppercase, space, empty), and the on-disk blob name is a fixed constant (contextBlobName) — no attacker-supplied filename is ever joined. The hostile-id matrix {"../evil","sub/../../etc","SUB-UPPER","has space","","a/b"} is test-pinned in both the local and S3 backends. The one free-form field a submission carries (DisplayName) is charset-constrained by displayNameRE and rejects control chars (submit_test.go "control chars" case).

5. K8s object names — validated at every cluster write.

ValidateServerName / ValidateSystemServerName (^[a-z0-9-]{3,32}$, no leading/trailing dash, reserved-name set) and ValidateHostname (dnsLabelRE, single label under the configured root domain) gate every create/patch. cluster.go documents the invariant: "there is no free-form YAML path — every field is a typed, validated value," so no raw CRD field can be smuggled through a create/patch body.

6. Numeric / enum — fail-closed.

resolveResources routes every quantity (memory, resources.cpu/memory, requests) through parsePositiveQuantity, which rejects q.Sign() <= 0 (negative and zero) with a field-named 400, plus a request>limit guard; parseStorageSize carries the same guard. Enums are closed sets: parseAutostartPolicy (ownerOnly/public/allowlist), the access-action switch, and the image ImageAdmitted allow-list (no free image string).

Verdict

Bad data does not pass. Every dangerous sink is fail-closed — including the break-glass / internal-face surface, where the hypothesised mc_uuid/OP-create → RCON paths were traced and found not to exist (parameterized DB, not RCON). Both text → RCON vectors — structured (handlers_access) and free-text (handleCommand) — are mutation-pinned by their named negative tests. No gap was found and no production code changed; the honest result of "故意加错误数据" is that the input surface rejects it.

Scope is deliberately bounded to the dangerous sinks and the newest (break-glass) code, not an exhaustive fuzz of all ~80 handlers — the claim proven is "every place user input reaches a dangerous sink validates before the sink," by trace plus two mutations, not "every handler was fuzzed."

7.7 KiB Raw Permalink Blame History