fix: validate relay URL before calling load() to prevent remote DoS via unhandled rejection #22

Merged
matt merged 1 commit from mailship-iij-post-notify-with-non-wss-relay-url-crash-655 into main 2026-09-18 15:38:47 +00:00
Collaborator

mailship-iij

Summary: A confirmed subscriber can crash the entire server with a single POST to /notify/:id by supplying a non-ws:// relay URL. The route handler passes the attacker-controlled relay straight to @welshman/net's load(), whose internal batcher throws Invalid relay url asynchronously (inside a setTimeout timer). The error escapes the route's try/catch as an unhandled rejection, and process.on('unhandledRejection') calls process.exit(1). Two interdependent fixes land: early input validation and a hardened global rejection handler.

Changes:

  1. Validate relay scheme before calling load() (src/server.ts). The route handler now checks isRelayUrl() from @welshman/util — the same function the library uses internally — and returns 400 with an explanatory error for any URL that isn't ws:// or wss://. This prevents the bad URL from ever reaching the batcher.

  2. Don't process.exit(1) on unhandledRejection (src/index.ts). The global handler now logs and continues instead of killing the process. Defence in depth: if any other async error escapes a try/catch (from any dependency), the server stays up.

  3. Test (test/notify-non-wss-relay-crash.test.ts). Starts an ephemeral server, creates a confirmed subscription, POSTs with http://127.0.0.1:9, and asserts a 400 response with the expected error message.

How to test: pnpm test:unit runs the full suite (17 tests across 6 files). For manual verification, curl -X POST -H 'Content-Type: application/json' -d '{"id":"any","relay":"http://127.0.0.1:9"}' http://localhost:3000/notify/<sub-id> previously killed the process; it now returns {"error":"Invalid relay URL. Only wss:// or ws:// relays are supported."} and the server stays running.

mailship-iij **Summary:** A confirmed subscriber can crash the entire server with a single POST to `/notify/:id` by supplying a non-ws:// relay URL. The route handler passes the attacker-controlled relay straight to `@welshman/net`'s `load()`, whose internal batcher throws `Invalid relay url` asynchronously (inside a `setTimeout` timer). The error escapes the route's `try/catch` as an unhandled rejection, and `process.on('unhandledRejection')` calls `process.exit(1)`. Two interdependent fixes land: early input validation and a hardened global rejection handler. **Changes:** 1. **Validate relay scheme before calling `load()`** (`src/server.ts`). The route handler now checks `isRelayUrl()` from `@welshman/util` — the same function the library uses internally — and returns 400 with an explanatory error for any URL that isn't `ws://` or `wss://`. This prevents the bad URL from ever reaching the batcher. 2. **Don't `process.exit(1)` on unhandledRejection** (`src/index.ts`). The global handler now logs and continues instead of killing the process. Defence in depth: if any other async error escapes a `try/catch` (from any dependency), the server stays up. 3. **Test** (`test/notify-non-wss-relay-crash.test.ts`). Starts an ephemeral server, creates a confirmed subscription, POSTs with `http://127.0.0.1:9`, and asserts a 400 response with the expected error message. **How to test:** `pnpm test:unit` runs the full suite (17 tests across 6 files). For manual verification, `curl -X POST -H 'Content-Type: application/json' -d '{"id":"any","relay":"http://127.0.0.1:9"}' http://localhost:3000/notify/<sub-id>` previously killed the process; it now returns `{"error":"Invalid relay URL. Only wss:// or ws:// relays are supported."}` and the server stays running.
hudson added 1 commit 2026-09-18 14:41:15 +00:00
fix: validate relay URL before calling load() and soften unhandledRejection handler
All checks were successful
CI / checks (pull_request) Successful in 34s
0a5b7ad14d
POST /notify/:id accepted an attacker-supplied relay URL with a non-ws://
scheme (e.g. http://…). The URL was passed straight to @welshman/net's
load(), whose internal batcher throws 'Invalid relay url' asynchronously
(via setTimeout). That error escaped the route's try/catch as an unhandled
rejection, triggering process.exit(1) in src/index.ts — the entire server
died. Any registered subscriber could crash the service repeatedly.

Two fixes applied (either alone breaks the attack):

1. Validate relay scheme in the route handler before calling load()
   (src/server.ts). Uses isRelayUrl() from @welshman/util, which accepts
   only wss:// and ws:// schemes. Returns 400 immediately for invalid
   URLs, preventing the bad URL from ever reaching the batcher.

2. Soften process.on('unhandledRejection') in src/index.ts to log and
   continue instead of calling process.exit(1). Defence in depth: if any
   other async error escapes a try/catch, the server stays up.

Test: test/notify-non-wss-relay-crash.test.ts — starts an ephemeral
server, creates a confirmed subscription, POSTs with http://127.0.0.1:9,
expects 400 with an error message.
matt approved these changes 2026-09-18 15:38:43 +00:00
matt merged commit b1a8258cfa into main 2026-09-18 15:38:47 +00:00
matt deleted branch mailship-iij-post-notify-with-non-wss-relay-url-crash-655 2026-09-18 15:39:23 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: matt/mailship#22
No description provided.