Unverified Commit 3338d6f0 authored by Minseong Choi's avatar Minseong Choi 💬
Browse files

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.
parent a4779186

internal/api/handlers_hasjoined.go

+9 −3
Changes for internal/api/handlers_hasjoined.go: 9 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -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}$`)

internal/api/handlers_hasjoined_test.go

+25 −6
Changes for internal/api/handlers_hasjoined_test.go: 25 added lines, 6 removed lines.
Original line number Diff line number Diff line
@@ -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)

		}

	})

}