Skip to content

fix(security): WS rate-limit XFF bypass + auth gate on public-keys/all#115

Merged
ralyodio merged 1 commit into
profullstack:masterfrom
AranitGjon:illyria/ugig-452fba61d3
Jun 29, 2026
Merged

fix(security): WS rate-limit XFF bypass + auth gate on public-keys/all#115
ralyodio merged 1 commit into
profullstack:masterfrom
AranitGjon:illyria/ugig-452fba61d3

Conversation

@AranitGjon

Copy link
Copy Markdown
Contributor

Fixes two security issues raised in the testers thread.

1. WebSocket rate-limit bypass (MEDIUM) — server.js

getClientIp() used the leftmost X-Forwarded-For value, which is client-controlled. An attacker can send a new fake X-Forwarded-For per connection to get a fresh "IP" each time and completely bypass the per-IP WebSocket connection limit (WS_MAX_CONNECTIONS_PER_IP).

Fix mirrors the existing trusted-proxy logic in src/lib/server/rate-limiter.js:

  • when TRUSTED_PROXY_COUNT > 0, read the Nth-from-right XFF entry (only hops appended by infra you control are trustworthy);
  • otherwise fall back to X-Real-IP then the raw socket peer address — never the spoofable leftmost XFF.

2. Unauthenticated user enumeration — GET /api/crypto/public-keys/all

The endpoint used the service-role key (bypasses RLS) and had no auth check, so any anonymous request returned every user_id + public key — full user-base enumeration.

Fix adds the same cookie/JWT authenticateUser gate already used by the sibling GET /api/crypto/public-keys and returns 401 when unauthenticated. The only legitimate caller (src/lib/crypto/key-sync-service.js) is an authenticated client, so no functional change for real users. Public keys remain non-secret; the membership list no longer leaks.

Both changes are minimal, follow existing repo patterns, and pass node --check.

@ralyodio ralyodio merged commit 3eeb961 into profullstack:master Jun 29, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants