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 <hello@matteocollina.com>
PR-URL: https://github.com/nodejs/node/pull/64545
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

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() {

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();