Denial of Service (unhandled socket error leading to potential process crash)

HIGH
nodejs/node
Commit: 1523d66a695a
Affected: Pre-25.9.0 (versions prior to this commit)
2026-04-05 11:38 UTC

Description

The commit fixes a race where socket errors emitted in the early window between onSocket and onSocketNT could be unhandled if the user had not yet attached a request error handler. Previously, an error could be emitted on the socket before the request’s error handling was wired up, potentially causing unhandled error events and denial of service (process crash or degraded service). The fix attaches a synchronous socket error listener in onSocket, marks the socket with _httpMessage, and forwards errors to the request. It also guards the _destroy path in onSocketNT to avoid double-firing if the early listener already emitted the error. Additionally, the changes in net.js ensure synchronous destruction of sockets on various error paths (e.g., IP block, connect errors, timeouts) rather than deferring to the next tick, reducing the window during which such errors could be mishandled. The test mentions the case where a blocklist or synchronous lookup error could emit before the request error handler is set up and asserts the prevention of double-emission. Vulnerability type: Denial of Service (DoS) risk from unhandled socket errors leaking out before user error handlers are attached; potential crash due to unhandled error events. Affected versions: pre-25.9.0 (versions prior to this patch)

Proof of Concept

#!/usr/bin/env node // PoC for DoS vulnerability in http client: unhandled socket error before user error handler is attached // Requires a vulnerable Node.js version (pre-25.9.0) where the early socket error could be emitted // without a user error listener on the ClientRequest. const http = require('http'); const net = require('net'); // Step 1: Create a BlockList and block 127.0.0.1 to trigger ERR_IP_BLOCKED in the socket path const BlockList = net.BlockList; const bl = new BlockList(); bl.addAddress('127.0.0.1'); // Step 2: Monkey-patch net.connect to attach our blocklist to sockets globally const originalConnect = net.connect; net.connect = function(...args) { const sock = originalConnect.apply(this, args); // Attach the blocklist so the socket checks if the destination is blocked sock.blockList = bl; return sock; }; // Step 3: Use a custom DNS lookup that resolves to 127.0.0.1 to force the blocked path const options = { hostname: 'example.invalid', port: 80, path: '/', method: 'GET', lookup: (hostname, opts, cb) => cb(null, '127.0.0.1', 4) }; // Step 4: Do not attach any error handler to the ClientRequest http.request(options, (res) => { res.on('data', () => {}); res.on('end', () => {}); }).end();

Commit Details

Author: RajeshKumar11

Date: 2026-02-16 08:26 UTC

Message:

http: attach error handler to socket synchronously in onSocket Between onSocket and onSocketNT, the socket had no error handler, meaning any errors emitted during that window (e.g. from a blocklist check or custom lookup) would be unhandled even if the user had set up a request error handler. Fix this by attaching socketErrorListener synchronously in onSocket, setting socket._httpMessage so the listener can forward errors to the request. The _destroy path in onSocketNT is also guarded to prevent double-firing if socketErrorListener already emitted the error. Fixes: https://github.com/nodejs/node/issues/48771 Refs: https://github.com/nodejs/node/pull/61658 PR-URL: https://github.com/nodejs/node/pull/61770 Refs: https://github.com/nodejs/node/issues/48771 Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Triage Assessment

Vulnerability Type: Denial of Service

Confidence: HIGH

Reasoning:

The commit ensures socket errors emitted during the initial window are not unhandled by attaching a synchronous error listener and forwarding errors to the request. This prevents unhandled errors (which could crash the process or lead to denial of service or information disclosure) and fixes a race where errors could bypass the user's error handler.

Verification Assessment

Vulnerability Type: Denial of Service (unhandled socket error leading to potential process crash)

Confidence: HIGH

Affected Versions: Pre-25.9.0 (versions prior to this commit)

Code Diff

diff --git a/lib/_http_client.js b/lib/_http_client.js index 68fec845695f26..1358a441c15f90 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -571,7 +571,7 @@ function socketErrorListener(err) { if (req) { // For Safety. Some additional errors might fire later on // and we need to make sure we don't double-fire the error event. - req.socket._hadError = true; + socket._hadError = true; emitErrorEvent(req, err); } @@ -910,7 +910,6 @@ function tickOnSocket(req, socket) { parser.joinDuplicateHeaders = req.joinDuplicateHeaders; parser.onIncoming = parserOnIncomingClient; - socket.on('error', socketErrorListener); socket.on('data', socketOnData); socket.on('end', socketOnEnd); socket.on('close', socketCloseListener); @@ -949,8 +948,15 @@ function listenSocketTimeout(req) { } ClientRequest.prototype.onSocket = function onSocket(socket, err) { - // TODO(ronag): Between here and onSocketNT the socket - // has no 'error' handler. + // Attach the error listener synchronously so that any errors emitted on + // the socket before onSocketNT runs (e.g. from a blocklist check or other + // next-tick error) are forwarded to the request and can be caught by the + // user's error handler. socketErrorListener requires socket._httpMessage + // to be set so we set it here too. + if (socket && !err) { + socket._httpMessage = this; + socket.on('error', socketErrorListener); + } process.nextTick(onSocketNT, this, socket, err); }; @@ -962,8 +968,10 @@ function onSocketNT(req, socket, err) { if (!req.aborted && !err) { err = new ConnResetException('socket hang up'); } - // ERR_PROXY_TUNNEL is handled by the proxying logic - if (err && err.code !== 'ERR_PROXY_TUNNEL') { + // ERR_PROXY_TUNNEL is handled by the proxying logic. + // Skip if the error was already emitted by the early socketErrorListener. + if (err && err.code !== 'ERR_PROXY_TUNNEL' && + !socket?._hadError) { emitErrorEvent(req, err); } req._closed = true; diff --git a/lib/net.js b/lib/net.js index cbde238ba0a2ce..e22ef4bfc4bff0 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1125,7 +1125,7 @@ function internalConnect( err = checkBindError(err, localPort, self._handle); if (err) { const ex = new ExceptionWithHostPort(err, 'bind', localAddress, localPort); - process.nextTick(emitErrorAndDestroy, self, ex); + self.destroy(ex); return; } } @@ -1135,7 +1135,7 @@ function internalConnect( if (addressType === 6 || addressType === 4) { if (self.blockList?.check(address, `ipv${addressType}`)) { - process.nextTick(emitErrorAndDestroy, self, new ERR_IP_BLOCKED(address)); + self.destroy(new ERR_IP_BLOCKED(address)); return; } const req = new TCPConnectWrap(); @@ -1167,20 +1167,12 @@ function internalConnect( } const ex = new ExceptionWithHostPort(err, 'connect', address, port, details); - process.nextTick(emitErrorAndDestroy, self, ex); + self.destroy(ex); } else if ((addressType === 6 || addressType === 4) && hasObserver('net')) { startPerf(self, kPerfHooksNetConnectContext, { type: 'net', name: 'connect', detail: { host: address, port } }); } } -// Helper function to defer socket destruction to the next tick. -// This ensures that error handlers have a chance to be set up -// before the error is emitted, particularly important when using -// http.request with a custom lookup function. -function emitErrorAndDestroy(self, err) { - self.destroy(err); -} - function internalConnectMultiple(context, canceled) { clearTimeout(context[kTimeout]); @@ -1194,11 +1186,11 @@ function internalConnectMultiple(context, canceled) { // All connections have been tried without success, destroy with error if (canceled || context.current === context.addresses.length) { if (context.errors.length === 0) { - process.nextTick(emitErrorAndDestroy, self, new ERR_SOCKET_CONNECTION_TIMEOUT()); + self.destroy(new ERR_SOCKET_CONNECTION_TIMEOUT()); return; } - process.nextTick(emitErrorAndDestroy, self, new NodeAggregateError(context.errors)); + self.destroy(new NodeAggregateError(context.errors)); return; } diff --git a/test/parallel/test-http-request-lookup-error-catchable.js b/test/parallel/test-http-request-lookup-error-catchable.js index 905f841c77c096..242f6ad45a553f 100644 --- a/test/parallel/test-http-request-lookup-error-catchable.js +++ b/test/parallel/test-http-request-lookup-error-catchable.js @@ -13,8 +13,8 @@ const net = require('net'); // 2. The lookup returns an IP that triggers a synchronous error (e.g., blockList) // 3. The error is emitted before http's error handler is set up (via nextTick) // -// The fix defers socket.destroy() calls in internalConnect to the next tick, -// giving http.request() time to set up its error handlers. +// The fix attaches socketErrorListener synchronously in onSocket so that +// socket errors are forwarded to the request before onSocketNT runs. const blockList = new net.BlockList(); blockList.addAddress(common.localhostIPv4);
← Back to Alerts View on GitHub →