From 84e367579ef43e0ae268463cc45c16a5992f38fb Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Thu, 16 Jul 2026 19:51:33 +0200 Subject: [PATCH] https: limit proxy CONNECT response headers Apply the configured maximum header size while reading CONNECT responses. Rearm the readable listener only once per read pass to avoid unbounded buffer and listener growth. Signed-off-by: Matteo Collina PR-URL: https://github.com/nodejs/node/pull/64545 Reviewed-By: Tim Perry Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- lib/https.js | 35 ++++++++++----- ...st-https-proxy-request-header-overflow.mjs | 45 +++++++++++++++++++ 2 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 test/client-proxy/test-https-proxy-request-header-overflow.mjs diff --git a/lib/https.js b/lib/https.js index 6ae2e5d8a21..75ec8bb3f89 100644 --- a/lib/https.js +++ b/lib/https.js @@ -70,6 +70,7 @@ let debug = require('internal/util/debuglog').debuglog('https', (fn) => { debug = fn; }); const net = require('net'); +const { Buffer } = require('buffer'); const { URL, urlToHttpOptions, isURL } = require('internal/url'); const { validateObject } = require('internal/validators'); const { isIP } = require('internal/net'); @@ -204,6 +205,7 @@ function getTunnelConfigForProxiedHttps(agent, reqOptions) { const result = { __proto__: null, proxyTunnelPayload: payload, + maxHeaderSize: reqOptions.maxHeaderSize || getOptionValue('--max-http-header-size'), requestOptions: { // Options used for the request sent after the tunnel is established. __proto__: null, servername: reqOptions.servername || ipType ? undefined : reqOptions.host, @@ -215,17 +217,17 @@ function getTunnelConfigForProxiedHttps(agent, reqOptions) { }; function establishTunnel(agent, socket, options, tunnelConfig, afterSocket) { - const { proxyTunnelPayload } = tunnelConfig; + const { proxyTunnelPayload, maxHeaderSize } = tunnelConfig; // By default, the socket is in paused mode. Read to look for the 200 // connection established response. function read() { let chunk; while ((chunk = socket.read()) !== null) { - if (onProxyData(chunk) !== -1) { - break; + if (onProxyData(chunk)) { + return; } } - socket.on('readable', read); + socket.once('readable', read); } function cleanup() { @@ -243,14 +245,25 @@ function establishTunnel(agent, socket, options, tunnelConfig, afterSocket) { // Read the headers from the chunks and check for the status code. If it fails we // clean up the socket and return an error. Otherwise we establish the tunnel. - let buffer = ''; + let buffer; function onProxyData(chunk) { - const str = chunk.toString(); - debug('onProxyData', str); - buffer += str; + debug('onProxyData', chunk.length); + buffer = buffer === undefined ? chunk : + Buffer.concat([buffer, chunk], buffer.length + chunk.length); const headerEndIndex = buffer.indexOf('\r\n\r\n'); - if (headerEndIndex === -1) return headerEndIndex; - const statusLine = buffer.substring(0, buffer.indexOf('\r\n')); + const headerLength = headerEndIndex === -1 ? + buffer.length : headerEndIndex + 4; + if (headerLength > maxHeaderSize) { + debug(`Proxy response headers exceed ${maxHeaderSize} bytes, cleaning up`); + cleanup(); + const err = new ERR_PROXY_TUNNEL( + `Proxy response headers exceeded ${maxHeaderSize} bytes`); + afterSocket(err, socket); + return true; + } + if (headerEndIndex === -1) return false; + const statusLineEndIndex = buffer.indexOf('\r\n'); + const statusLine = buffer.subarray(0, statusLineEndIndex).toString(); const statusCode = statusLine.split(' ')[1]; if (statusCode !== '200') { debug(`onProxyData receives ${statusCode}, cleaning up`); @@ -292,7 +305,7 @@ function establishTunnel(agent, socket, options, tunnelConfig, afterSocket) { }); tunneldSocket.on('error', onTLSHandshakeError); } - return headerEndIndex; + return true; } function onProxyEnd() { diff --git a/test/client-proxy/test-https-proxy-request-header-overflow.mjs b/test/client-proxy/test-https-proxy-request-header-overflow.mjs new file mode 100644 index 00000000000..2ffc215bafa --- /dev/null +++ b/test/client-proxy/test-https-proxy-request-header-overflow.mjs @@ -0,0 +1,45 @@ +// This tests that the proxy server cannot send unbounded incomplete headers +// while establishing a CONNECT tunnel. + +import * as common from '../common/index.mjs'; +import assert from 'node:assert'; +import { once } from 'events'; +import http from 'node:http'; +import { runProxiedRequest } from '../common/proxy-server.js'; + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const proxy = http.createServer(); +proxy.on('connect', common.mustCall((req, socket) => { + socket.write('HTTP/1.1 200 Connection Established\r\n'); + + const interval = setInterval(() => { + if (socket.destroyed) { + clearInterval(interval); + return; + } + socket.write('x'.repeat(100)); + }, 10); + socket.on('close', () => clearInterval(interval)); + socket.on('error', () => clearInterval(interval)); +}, 1)); +proxy.listen(0); +await once(proxy, 'listening'); + +const { code, signal, stderr, stdout } = await runProxiedRequest({ + NODE_USE_ENV_PROXY: 1, + REQUEST_URL: 'https://localhost:1/test', + HTTPS_PROXY: `http://localhost:${proxy.address().port}`, +}, ['--max-http-header-size=1024']); + +assert.match( + stderr, + /ERR_PROXY_TUNNEL.*Proxy response headers exceeded 1024 bytes/, +); +assert.doesNotMatch(stderr, /MaxListenersExceededWarning/); +assert.strictEqual(stdout.trim(), ''); +assert.strictEqual(code, 0); +assert.strictEqual(signal, null); + +proxy.close();