fix: validate relay URL before calling load() to prevent remote DoS via unhandled rejection #22
Loading…
Reference in a new issue
No description provided.
Delete branch "mailship-iij-post-notify-with-non-wss-relay-url-crash-655"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
mailship-iij
Summary: A confirmed subscriber can crash the entire server with a single POST to
/notify/:idby supplying a non-ws:// relay URL. The route handler passes the attacker-controlled relay straight to@welshman/net'sload(), whose internal batcher throwsInvalid relay urlasynchronously (inside asetTimeouttimer). The error escapes the route'stry/catchas an unhandled rejection, andprocess.on('unhandledRejection')callsprocess.exit(1). Two interdependent fixes land: early input validation and a hardened global rejection handler.Changes:
Validate relay scheme before calling
load()(src/server.ts). The route handler now checksisRelayUrl()from@welshman/util— the same function the library uses internally — and returns 400 with an explanatory error for any URL that isn'tws://orwss://. This prevents the bad URL from ever reaching the batcher.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 atry/catch(from any dependency), the server stays up.Test (
test/notify-non-wss-relay-crash.test.ts). Starts an ephemeral server, creates a confirmed subscription, POSTs withhttp://127.0.0.1:9, and asserts a 400 response with the expected error message.How to test:
pnpm test:unitruns 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.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.