aboutsummaryrefslogtreecommitdiff
path: root/internal/admin
diff options
context:
space:
mode:
authorhistoria <[not public]>2026-09-18 15:42:23 -0400
committerhistoria <[not public]>2026-09-18 15:42:23 -0400
commit67e01aab85607b9e601185c7d62dff576af3bb43 (patch)
tree059134b2434eba7c14aa9d89f48c5a475feb0312 /internal/admin
parent7eab582d8a8ee2af1a6baae7baf0696c7005de2e (diff)
downloadthehouseoficarus-67e01aab85607b9e601185c7d62dff576af3bb43.tar.gz
fix: close map OOM, admin traversal, transport safespot, room rekey, and legacy payout recovery
Diffstat (limited to 'internal/admin')
-rw-r--r--internal/admin/api_players.go18
-rw-r--r--internal/admin/api_players_path_test.go87
-rw-r--r--internal/admin/server.go6
3 files changed, 110 insertions, 1 deletions
diff --git a/internal/admin/api_players.go b/internal/admin/api_players.go
index 66142c5..8ac0dd2 100644
--- a/internal/admin/api_players.go
+++ b/internal/admin/api_players.go
@@ -87,7 +87,23 @@ func (s *AdminServer) handlePlayerByID(w http.ResponseWriter, r *http.Request) {
}
if strings.HasPrefix(path, "account/") {
- s.getAccount(w, strings.TrimPrefix(path, "account/"))
+ acct := strings.TrimPrefix(path, "account/")
+ if err := player.ValidName(acct); err != nil {
+ http.Error(w, `{"error":"invalid account id"}`, http.StatusBadRequest)
+ return
+ }
+ s.getAccount(w, acct)
+ return
+ }
+
+ // Validate before the first join into players/characters/<name>.yaml.
+ // ServeMux cleans the ESCAPED path but handlers read the DECODED
+ // r.URL.Path, so a percent-encoded segment like %2e%2e%2f reaches this
+ // handler as "../" and would otherwise escape the data dir (and bypass
+ // the password_hash redaction in getAccount). ValidName is the same
+ // invariant every other name join in the codebase enforces.
+ if err := player.ValidName(path); err != nil {
+ http.Error(w, `{"error":"invalid player id"}`, http.StatusBadRequest)
return
}
diff --git a/internal/admin/api_players_path_test.go b/internal/admin/api_players_path_test.go
new file mode 100644
index 0000000..dca156e
--- /dev/null
+++ b/internal/admin/api_players_path_test.go
@@ -0,0 +1,87 @@
+package admin
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "thehouseoficarus/internal/config"
+ "thehouseoficarus/internal/player"
+)
+
+// TestPlayerAPICannotEscapeData: /api/players/{name} joins the raw URL
+// remainder into players/characters/<name>.yaml. ServeMux cleans the ESCAPED
+// path but handlers read the DECODED r.URL.Path, so a percent-encoded segment
+// (%2e%2e%2f) arrives as "../" and would otherwise read arbitrary *.yaml —
+// including account files, bypassing the password_hash redaction that
+// getAccount applies. The route must validate the name before the first join.
+func TestPlayerAPICannotEscapeData(t *testing.T) {
+ root := t.TempDir()
+ dir := filepath.Join(root, "data")
+ for _, sub := range []string{"players/accounts", "players/characters"} {
+ if err := os.MkdirAll(filepath.Join(dir, sub), 0o700); err != nil {
+ t.Fatal(err)
+ }
+ }
+ accounts := player.NewAccountStore(dir)
+ if err := accounts.SaveAccount(&player.Account{Name: "auditor", Admin: true, PasswordHash: "argon2id$auditor-hash"}); err != nil {
+ t.Fatal(err)
+ }
+ if err := accounts.SaveAccount(&player.Account{Name: "victim", PasswordHash: "argon2id$victim-hash"}); err != nil {
+ t.Fatal(err)
+ }
+ victim := player.New("victim")
+ victim.Account = "victim"
+ if err := accounts.SaveCharacter(victim); err != nil {
+ t.Fatal(err)
+ }
+ // A canary file outside data/ that traversal could otherwise reach.
+ if err := os.WriteFile(filepath.Join(root, "outside.yaml"), []byte("secret: canary\n"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+
+ s, err := NewServer(config.Default(), false, accounts, nil, nil, nil, nil, dir,
+ nil, nil, nil, nil, filepath.Join(dir, "config.yaml"),
+ func(fn func()) bool { fn(); return true })
+ if err != nil {
+ t.Fatal(err)
+ }
+ cookieRec := httptest.NewRecorder()
+ s.setAuthCookie(cookieRec, "auditor")
+ cookie := cookieRec.Result().Cookies()[0]
+
+ get := func(target string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(http.MethodGet, target, nil)
+ req.AddCookie(cookie)
+ rec := httptest.NewRecorder()
+ s.ServeHTTP(rec, req)
+ return rec
+ }
+
+ // Encoded traversal to an account file must be rejected, and the raw
+ // password hash must never appear in the response.
+ rec := get("/api/players/%2e%2e%2faccounts%2fvictim")
+ if rec.Code != http.StatusBadRequest {
+ t.Fatalf("encoded traversal: status %d body %s", rec.Code, rec.Body.String())
+ }
+ if strings.Contains(rec.Body.String(), "victim-hash") {
+ t.Fatal("encoded traversal leaked a password hash")
+ }
+
+ // Traversal out of the data dir to a canary must be rejected.
+ rec = get("/api/players/%2e%2e%2f%2e%2e%2foutside")
+ if rec.Code != http.StatusBadRequest {
+ t.Fatalf("outside traversal: status %d body %s", rec.Code, rec.Body.String())
+ }
+ if strings.Contains(rec.Body.String(), "canary") {
+ t.Fatal("encoded traversal read a file outside the data dir")
+ }
+
+ // A legitimate lookup still works.
+ if rec = get("/api/players/victim"); rec.Code != http.StatusOK {
+ t.Fatalf("legit player lookup: status %d body %s", rec.Code, rec.Body.String())
+ }
+}
diff --git a/internal/admin/server.go b/internal/admin/server.go
index 4ea22d8..c13fe96 100644
--- a/internal/admin/server.go
+++ b/internal/admin/server.go
@@ -389,6 +389,12 @@ func (s *AdminServer) checkAuth(r *http.Request) bool {
return false
}
account := parts[0]
+ // The account name is attacker-controlled (it comes from the cookie) and
+ // is joined into players/accounts/<account>.yaml by isAdminAccount below,
+ // so reject path-like names before the first join.
+ if err := player.ValidName(account); err != nil {
+ return false
+ }
issued, err := strconv.ParseInt(parts[1], 10, 64)
if err != nil {
return false