diff --git a/src/index.ts b/src/index.ts index 8f9dd20..4c92819 100644 --- a/src/index.ts +++ b/src/index.ts @@ -7,7 +7,9 @@ import { registerSubscription } from './worker/index.js' process.on('unhandledRejection', (error: Error) => { console.error('Unhandled rejection:', error.stack) - process.exit(1) + // Do not process.exit(1) — an async rejection from a library's internal + // timer (e.g. @welshman/net's batcher) would let an attacker crash the + // entire server with a single malformed request. Log and continue. }) process.on('uncaughtException', (error: Error) => { @@ -15,6 +17,7 @@ process.on('uncaughtException', (error: Error) => { process.exit(1) }) + migrate().then(async () => { server.listen(PORT, () => { console.log('Running on port', PORT) diff --git a/src/server.ts b/src/server.ts index c7acb43..7cf5fdb 100644 --- a/src/server.ts +++ b/src/server.ts @@ -6,7 +6,7 @@ import { render } from './templates.js' import { confirmSubscriptionAction, unsubscribeAction, registerSubscription, ActionError } from './actions.js' import { getSubscriptionById, insertEvent, getSubscriptionByKey, getSubscriptionByPubkey } from './database.js' import { load } from '@welshman/net' -import { getIdFilters } from '@welshman/util' +import { getIdFilters, isRelayUrl } from '@welshman/util' import crypto from 'crypto' import { verifyEvent } from 'nostr-tools/pure' @@ -263,6 +263,15 @@ addRoute('post', '/notify/:id', async (req: Request, res: Response) => { return res.status(400).json({ error: 'id and relay are required' }) } + // Reject non-ws:// relay URLs. load() from @welshman/net throws + // Invalid relay url asynchronously inside a batcher timer, which + // escapes the route's try/catch and becomes an unhandledRejection + // that would crash the server. Validate early to avoid calling + // load() with an unsupported scheme. + if (!isRelayUrl(relay)) { + return res.status(400).json({ error: 'Invalid relay URL. Only wss:// or ws:// relays are supported.' }) + } + const sub = await getSubscriptionById(req.params.id) if (!sub) { diff --git a/test/notify-non-wss-relay-crash.test.ts b/test/notify-non-wss-relay-crash.test.ts new file mode 100644 index 0000000..e6e87a5 --- /dev/null +++ b/test/notify-non-wss-relay-crash.test.ts @@ -0,0 +1,82 @@ +// POST /notify with non-wss relay URL crashes the server (remote DoS) +// +// Bug: When POST /notify/:id receives a relay URL with a non-wss scheme +// (e.g. http://…), the handler calls `load()` from @welshman/net. Inside +// `load`, the batcher schedules an async `_execute` via setTimeout(200ms). +// When `getAdapter` throws `Invalid relay url`, the error escapes as an +// unhandledPromiseRejection because the batcher's `_execute` async function +// is called from setTimeout with no `.catch()`. The global +// `process.on('unhandledRejection')` handler in `src/index.ts` then calls +// `process.exit(1)`, killing the entire server. +// +// Fix applied (2 of 3 fixes): +// 1. Validate relay scheme before calling load() — the route handler now +// checks isRelayUrl() and returns 400 for non-ws:// schemes. +// 2. Don't process.exit(1) on unhandledRejection — log and continue +// (defence in depth for any other async edge-case). + +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import * as db from '../src/database.js' +import { server } from '../src/server.js' +import { createServer, type Server } from 'http' + +// Partially mock @welshman/net so that `load()` returns an empty array, +// preventing real relay connections during the test, while preserving all +// other exports from the library. +vi.mock('@welshman/net', async (importOriginal) => { + const actual = await importOriginal() + return { + ...(actual as Record), + load: vi.fn().mockResolvedValue([]), + } +}) + +describe('notify_non_wss_relay', () => { + let httpServer: Server + let baseUrl: string + let subId: string + + beforeAll(async () => { + await db.migrate() + + // Create and confirm a subscription we can use for the notify call + const pubkey = 'nws-test-pk-' + Date.now() + const email = 'nws-test-' + Date.now() + '@example.com' + const sub = await db.insertSubscription(pubkey, email, 'daily') + const confirmed = await db.confirmSubscription(sub.key) + subId = confirmed.id + + // Start the express server on a random available port + await new Promise((resolve) => { + httpServer = createServer(server) + httpServer.listen(0, () => { + const addr = httpServer.address() + if (addr && typeof addr === 'object') { + baseUrl = `http://localhost:${addr.port}` + } + resolve() + }) + }) + }) + + afterAll(async () => { + httpServer?.close() + }) + + it('rejects non-wss relay URL with 400 instead of crashing the server', async () => { + const res = await fetch(`${baseUrl}/notify/${subId}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + id: 'nonexistent-' + Date.now(), + relay: 'http://127.0.0.1:9', + }), + }) + + expect(res.status).toBe(400) + + const body = await res.json() + expect(body).toHaveProperty('error') + expect(body.error).toMatch(/Invalid relay/) + }) +}) \ No newline at end of file