diff --git a/src/server/ClientMsgRateLimiter.ts b/src/server/ClientMsgRateLimiter.ts index 986149da9..5177eb79c 100644 --- a/src/server/ClientMsgRateLimiter.ts +++ b/src/server/ClientMsgRateLimiter.ts @@ -3,18 +3,14 @@ import { ClientID } from "../core/Schemas"; const INTENTS_PER_SECOND = 10; const INTENTS_PER_MINUTE = 150; -const MAX_BYTES_PER_MINUTE = 25 * 1024; // 25KB/min per client -const MAX_INTENT_BYTES = 500; // intents are stored in turns, keep them small +const MAX_INTENT_SIZE = 500; +const TOTAL_BYTES = 2 * 1024 * 1024; // 2MB per client export type RateLimitResult = "ok" | "limit" | "kick"; -// Allow 3 winner messages per client since a player can rejoin and resend. -const MAX_WINNER_MSGS = 3; - interface ClientBucket { perSecond: RateLimiter; perMinute: RateLimiter; - bytesPerMinute: RateLimiter; - winnerMsgCount: number; + totalBytes: number; } export class ClientMsgRateLimiter { @@ -22,27 +18,27 @@ export class ClientMsgRateLimiter { check(clientID: ClientID, type: string, bytes: number): RateLimitResult { const bucket = this.getOrCreate(clientID); + bucket.totalBytes += bytes; - // Winner message contains stats for all players and can be large (100s of KB). - // It bypasses the byte rate limit but is strictly limited to one per client. - if (type === "winner") { - if (bucket.winnerMsgCount >= MAX_WINNER_MSGS) return "kick"; - bucket.winnerMsgCount++; - return "ok"; + if (bucket.totalBytes >= TOTAL_BYTES) return "kick"; + + if (type === "intent") { + // Intents are stored in turn history for the duration of the game, so + // oversized intents would accumulate and fill up server RAM. + // Intents are also sent to all players, so it increase outgoing + // data. + // Intents should never be larger than MAX_INTENT_SIZE, so we assume the client is malicious. + if (bytes > MAX_INTENT_SIZE) { + return "kick"; + } + if ( + !bucket.perSecond.tryRemoveTokens(1) || + !bucket.perMinute.tryRemoveTokens(1) + ) { + return "limit"; + } } - // Intents are stored in turn history for the duration of the game, so - // oversized intents would accumulate and fill up server RAM. - if (type === "intent" && bytes > MAX_INTENT_BYTES) return "kick"; - - if (!bucket.bytesPerMinute.tryRemoveTokens(bytes)) return "kick"; - - if ( - !bucket.perSecond.tryRemoveTokens(1) || - !bucket.perMinute.tryRemoveTokens(1) - ) - return "limit"; - return "ok"; } @@ -60,11 +56,7 @@ export class ClientMsgRateLimiter { tokensPerInterval: INTENTS_PER_MINUTE, interval: "minute", }), - bytesPerMinute: new RateLimiter({ - tokensPerInterval: MAX_BYTES_PER_MINUTE, - interval: "minute", - }), - winnerMsgCount: 0, + totalBytes: 0, }; this.buckets.set(clientID, bucket); return bucket; diff --git a/tests/server/ClientMsgRateLimiter.test.ts b/tests/server/ClientMsgRateLimiter.test.ts index 263464485..9732d3c40 100644 --- a/tests/server/ClientMsgRateLimiter.test.ts +++ b/tests/server/ClientMsgRateLimiter.test.ts @@ -5,7 +5,6 @@ const CLIENT_A = "clientA" as any; const CLIENT_B = "clientB" as any; const SMALL = 100; -const LARGE = 501; // over MAX_INTENT_BYTES describe("ClientMsgRateLimiter", () => { describe("intent messages", () => { @@ -14,11 +13,6 @@ describe("ClientMsgRateLimiter", () => { expect(limiter.check(CLIENT_A, "intent", SMALL)).toBe("ok"); }); - it("kicks on oversized intent", () => { - const limiter = new ClientMsgRateLimiter(); - expect(limiter.check(CLIENT_A, "intent", LARGE)).toBe("kick"); - }); - it("limits when per-second count exceeded", () => { const limiter = new ClientMsgRateLimiter(); for (let i = 0; i < 10; i++) { @@ -36,34 +30,46 @@ describe("ClientMsgRateLimiter", () => { }); }); - describe("winner messages", () => { - it("allows first winner message", () => { + describe("non-intent messages", () => { + it("does not rate-limit non-intent messages", () => { const limiter = new ClientMsgRateLimiter(); - expect(limiter.check(CLIENT_A, "winner", 50000)).toBe("ok"); + for (let i = 0; i < 20; i++) { + expect(limiter.check(CLIENT_A, "winner", 50)).toBe("ok"); + } }); - it("allows up to 3 winner messages", () => { + it("does not rate-limit ping messages", () => { const limiter = new ClientMsgRateLimiter(); - expect(limiter.check(CLIENT_A, "winner", 50000)).toBe("ok"); - expect(limiter.check(CLIENT_A, "winner", 50000)).toBe("ok"); - expect(limiter.check(CLIENT_A, "winner", 50000)).toBe("ok"); - expect(limiter.check(CLIENT_A, "winner", 50000)).toBe("kick"); - }); - - it("winner does not consume intent rate limit", () => { - const limiter = new ClientMsgRateLimiter(); - limiter.check(CLIENT_A, "winner", 50000); - expect(limiter.check(CLIENT_A, "intent", SMALL)).toBe("ok"); + for (let i = 0; i < 20; i++) { + expect(limiter.check(CLIENT_A, "ping", 50)).toBe("ok"); + } }); }); - describe("other messages", () => { - it("applies rate limiting to other message types", () => { + describe("total bytes limit", () => { + it("kicks when cumulative bytes reach 2MB", () => { const limiter = new ClientMsgRateLimiter(); - for (let i = 0; i < 10; i++) { - expect(limiter.check(CLIENT_A, "ping", 50)).toBe("ok"); + const chunkSize = 512 * 1024; // 512KB + // Send 3 chunks = 1.5MB, should be ok + for (let i = 0; i < 3; i++) { + expect(limiter.check(CLIENT_A, "other", chunkSize)).toBe("ok"); } - expect(limiter.check(CLIENT_A, "ping", 50)).toBe("limit"); + // 4th chunk pushes to 2MB, should kick + expect(limiter.check(CLIENT_A, "other", chunkSize)).toBe("kick"); + }); + + it("byte tracking is per client", () => { + const limiter = new ClientMsgRateLimiter(); + const almostFull = 2 * 1024 * 1024 - 1; + expect(limiter.check(CLIENT_A, "other", almostFull)).toBe("ok"); + // CLIENT_B should still be fine + expect(limiter.check(CLIENT_B, "other", 100)).toBe("ok"); + }); + + it("kicks on bytes regardless of message type", () => { + const limiter = new ClientMsgRateLimiter(); + const twoMB = 2 * 1024 * 1024; + expect(limiter.check(CLIENT_A, "intent", twoMB)).toBe("kick"); }); }); });