0a5b7ad14d
All checks were successful
CI / checks (pull_request) Successful in 34s
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.
82 lines
No EOL
2.9 KiB
TypeScript
82 lines
No EOL
2.9 KiB
TypeScript
// 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<string, unknown>),
|
|
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<void>((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/)
|
|
})
|
|
}) |