From 3338d6f0feeaf8ff240b265cb9acf49d6a79c681 Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Tue, 22 Sep 2026 13:21:43 +0900 Subject: [PATCH] fix(nano): drop oversized hasJoined parameters before asking sources username, serverId and ip were forwarded to every configured source at whatever length the caller sent, up to the megabyte net/http allows in a request line. Velocity never sends more than a 16-character name, a 41-character signed SHA-1 serverId and a textual IP address, so only a direct caller reaches those sizes, and each such request cost one oversized upstream call per source. Any of the three over 64 bytes is now answered 204 before a source is asked, the same as a missing username or serverId. 64 bytes still leaves room for a 16-character name in multi-byte UTF-8. The subtest behind this points a source that validates anything at the handler and sends missing and oversized fields, expecting 204 and zero upstream requests, then a well-formed login that gets 200. It replaces the old missing-username case, whose only source was unreachable, so the test passed even with the guard removed. Dropping the length check now fails it on the long username; dropping the whole guard fails it on the first missing field. --- internal/api/handlers_hasjoined.go | 12 +++++++--- internal/api/handlers_hasjoined_test.go | 31 ++++++++++++++++++++----- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/internal/api/handlers_hasjoined.go b/internal/api/handlers_hasjoined.go index 7454ebe..8eac3ff 100644 --- a/internal/api/handlers_hasjoined.go +++ b/internal/api/handlers_hasjoined.go @@ -116,13 +116,14 @@ func (a *API) handleHasJoined(w http.ResponseWriter, r *http.Request) { return } q := r.URL.Query() - username, serverID := q.Get("username"), q.Get("serverId") - if username == "" || serverID == "" { + username, serverID, ip := q.Get("username"), q.Get("serverId"), q.Get("ip") + if username == "" || serverID == "" || + len(username) > maxHasJoinedParam || len(serverID) > maxHasJoinedParam || len(ip) > maxHasJoinedParam { w.WriteHeader(http.StatusNoContent) return } - prof, src, failed := a.resolveHasJoined(r.Context(), username, serverID, q.Get("ip")) + prof, src, failed := a.resolveHasJoined(r.Context(), username, serverID, ip) if prof == nil { // With a source down, "nobody knows this player" is not established: its player may // be the one logging in. 503 makes Velocity report the auth servers as down and log @@ -188,6 +189,11 @@ func (a *API) handleHasJoined(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, prof) } +// maxHasJoinedParam bounds each query value before it is forwarded to every source. What +// Velocity sends fits with room to spare: a login name of at most 16 characters, a signed +// SHA-1 hex serverId of at most 41, a textual IP address. Only a direct caller sends more. +const maxHasJoinedParam = 64 + // mcUsernameRe is Minecraft's username charset — the trust boundary on a third-party // source's self-asserted profile name. var mcUsernameRe = regexp.MustCompile(`^[A-Za-z0-9_]{3,16}$`) diff --git a/internal/api/handlers_hasjoined_test.go b/internal/api/handlers_hasjoined_test.go index 459e080..3b769ce 100644 --- a/internal/api/handlers_hasjoined_test.go +++ b/internal/api/handlers_hasjoined_test.go @@ -338,13 +338,32 @@ func TestHasJoined(t *testing.T) { } }) - // Missing query fields → 204 without touching any source. - t.Run("missing username -> 204", func(t *testing.T) { + // A missing or oversized field is answered 204 before any source sees it. The source + // here validates anything it is asked, so only a request that never reaches it is a 204. + t.Run("missing or oversized query field -> 204 without asking a source", func(t *testing.T) { + var hits atomic.Int32 + src := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + _ = json.NewEncoder(w).Encode(map[string]any{"id": notchMojangID, "name": "Notch"}) + })) + t.Cleanup(src.Close) api := newTestAPI(newFakeRepo(), newFakeCluster()) - api.AuthSources = []AuthSource{{Tag: "mojang", URL: "http://127.0.0.1:0", Identity: true}} - w := do(api.InternalHandler(), "GET", "/session/minecraft/hasJoined?serverId=abc", "", nil) - if w.Code != http.StatusNoContent { - t.Fatalf("code = %d, want 204", w.Code) + api.AuthSources = []AuthSource{{Tag: "mojang", URL: src.URL, Identity: true}} + long := strings.Repeat("a", maxHasJoinedParam+1) + for _, query := range []string{ + "serverId=abc", + "username=Notch", + "username=" + long + "&serverId=abc", + "username=Notch&serverId=" + long, + "username=Notch&serverId=abc&ip=" + long, + } { + w := do(api.InternalHandler(), "GET", "/session/minecraft/hasJoined?"+query, "", nil) + if w.Code != http.StatusNoContent || hits.Load() != 0 { + t.Fatalf("%.40s: code = %d, source asked %d times; want 204 and 0", query, w.Code, hits.Load()) + } + } + if w := getHasJoined(api.InternalHandler(), "Notch", "abc"); w.Code != http.StatusOK { + t.Fatalf("a well-formed login: code = %d, want 200 (the source is live)", w.Code) } }) }