Skip to content

uWS backend (#1096): silent onUpgrade auth bypass, unconditional loopback IP on delegated routes, unbounded static-file buffering #1752

Description

@kriszyp

Summary

Exploratory QA against the new opt-in uWebSockets.js HTTP/WebSocket backend (#1096, HARPER_UWS_UDS/HARPER_UWS_HTTP, merged bd1dce0b1) found three independent, confirmed gaps — two security-shaped, one a resource-exhaustion vector. Both flags are default-off, so there's no live production exposure today, but all three are concrete gaps an operator would hit immediately upon opting in for the throughput win the PR advertises. Grouping them here since they're all on the same feature, found the same day, and likely worth reviewing together before any default flip or staging rollout.

All three reproduced on bd1dce0b1, npm run test:integration on each linked test file.


1. onUpgrade pre-handshake auth gate is silently inert under uWS

The PR's own text discloses that custom onUpgrade (server.upgrade()) listeners "aren't mirrored yet" under uWS — confirmed in code: server/serverHelpers/uwsServer.ts never touches onUpgrade/upgradeChains at all. The concrete, exercised consequence is worse than "a feature isn't implemented yet": it's a silent bypass with zero operator-facing signal.

Repro: a component registers server.upgrade((request, socket, head, next) => {...}) rejecting any WS handshake missing a custom X-App-Token header (401 + socket.destroy()), backed by a real server.ws(...) handler.

  • Node backend (control): header-less client → correctly rejected (401, socket closed). Header-present client → connects.
  • uWS backend (HARPER_UWS_HTTP=1): header-less client → connects successfully anyway, receives the welcome message the gate should have blocked.
  • Checked both startup output and continuous runtime logs across the whole run: no signal anywhere that a registered onUpgrade listener is incompatible with the active backend.

Root cause: onWebSocket's uws branch (server/http.ts ~1340-1367) wires the WS handler straight into uWS's native app.ws(), bypassing upgradeChains entirely.

Any app using server.upgrade() for its own pre-handshake authorization (IP allowlisting, custom pre-auth token check, etc. — a documented API, not an edge case) silently loses that protection the moment uWS is enabled.

Test: integrationTests/qa-scratch/qa531-uws-onupgrade-bypass.test.ts (drives both backends in one run).


2. Legacy Fastify-delegated routes see an unconditional 127.0.0.1 client IP under uWS

The PR states an explicit security property for the native path: "reads the real client IP from the TCP peer and only trusts X-Forwarded-For on the UDS path, so a direct client can't spoof loopback to satisfy local auth." This holds for the native httpChain path — but routes that fall through to the new injectToFastify() bridge (added by this same PR, for legacy Fastify-registered custom-function routes) get a mocked request whose IP is always 127.0.0.1, for every real client, with no forged header required — this isn't spoofing, it's a permanently wrong value.

Repro: two "local-admin-only" IP-gate routes side by side — one served by the native httpChain, one an unmatched legacy fastifyRoutes route (confirmed via a marker that it genuinely hit the delegation bridge, not the native path). Client explicitly bound its outbound socket to a distinct loopback address so a real, non-matching peer IP could be constructed.

  • Native path: correct real peer IP on both Node and uWS backends; forged X-Forwarded-For never trusted.
  • Delegated path under uWS: unconditionally 127.0.0.1, admitted:true, even with zero headers forged, for a real peer at a different address.

Root cause: injectToFastify() (server/http.ts ~1100-1116) forwards headers to fastify.inject() but never passes a remoteAddress option. Fastify v5's fastify.inject() is backed by light-my-request, whose MockSocket hardcodes remoteAddress = options.remoteAddress || '127.0.0.1' when unset; Harper never sets trustProxy, so request.ip just returns that hardcoded value.

Any app relying on a legacy Fastify-registered route for local-admin/IP-allowlist gating is unconditionally, permanently bypassed the moment uWS is enabled — no attacker action needed.

Suggested fix: injectToFastify() should pass remoteAddress: request.ip (the real value already resolved on the native request before delegation) through to fastify.inject()'s options.

Test: integrationTests/qa-scratch/qa533-uws-fastify-delegate-ip-trust.test.ts.


3. Static-file serving under uWS buffers 2.5-3.4× the file size in peak RSS, uncapped under concurrency

The PR discloses "static responses are buffered, not streamed" as a known tradeoff for typical assets. Measurement shows this understates the severity: it isn't one buffered copy per request, and the per-request cost multiplies with concurrency with no cap anywhere in the path.

Measured (byte-exact transfers throughout, no crashes — a resource curve, not a functional break):

Test Node RSS Δ uWS RSS Δ Node latency uWS latency
50MB file 46MB 123MB (~2.5×) 124ms 215ms
200MB file 0.2MB (streamed) 670MB (~3.35×) 267ms 1330ms
500MB file 41MB 1523MB (~3×) 580ms 4611ms
5× concurrent 500MB 20MB total 6932MB (2.77× a naive "5 full copies" estimate) 2.66s wall 21.8s wall
10× concurrent 500MB 3.5MB total 8905MB (peak RSS ~14.3GB) 5.36s wall 41.4s wall

Root cause: bufferSendStream() (server/http.ts, reached only via makeUwsHandler) drains the send-package SendStream into a chunks: Buffer[] and Buffer.concat()s it before responding — no size cap, no streaming fallback for large files.

On a typical 2-8GB container, a handful of concurrent requests to one moderately-large unauthenticated static asset (no auth required for static serving) is enough to threaten OOM. Memory also didn't fully drain between bursts in testing — sustained moderate load ratchets the high-water mark upward, not just spike-and-recover.

Suggested fix: stream static responses through uWS (the PR's own stated eventual direction) before considering a default flip, or add a size/concurrency-aware cap as a stopgap.

Test: integrationTests/qa-scratch/qa532-uws-static-buffering.test.ts (self-generates/self-deletes large fixtures; standalone regen helper generate-large-fixture.mjs retained).


Recommendation

Recommend prioritizing #1 (auth bypass) and #2 (IP-trust bypass) over #3 (resource curve) given the security shape, but all three are worth resolving — or at minimum documenting loudly with a startup-time warning — before uWS graduates past default-off or gets exercised on a staging cluster with real traffic.

— KrAIs 🤖 (exploratory QA, on Kris's behalf)

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:performancePerformance, latency, throughputarea:securitySecurity, TLS/certs, authentication, authorizationbugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions