From 11fdae4e532a5575ac9194fe37797b6d273645b0 Mon Sep 17 00:00:00 2001
From: "Shuowang (Wayne) Zhang" <shuowang.zhang@ibm.com>
Date: Tue, 11 Feb 2020 17:14:18 -0500
Subject: [PATCH] http: opt-in insecure HTTP header parsing

Backport 496736f

Original commit message:

    Allow insecure HTTP header parsing. Make clear it is insecure.

    See:
    - https://github.com/nodejs/node/pull/30553
    - https://github.com/nodejs/node/issues/27711#issuecomment-556265881
    - https://github.com/nodejs/node/issues/30515

    PR-URL: https://github.com/nodejs/node/pull/30567
    Backport-PR-URL: https://github.com/nodejs/node/pull/30473
    Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
    Reviewed-By: Anna Henningsen <anna@addaleax.net>
    Reviewed-By: Denys Otrishko <shishugi@gmail.com>
    Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/cli.md          | 12 ++++++++++++
 doc/node.1              |  7 +++++++
 lib/_http_client.js     |  7 +++++--
 lib/_http_common.js     | 13 ++++++++++++-
 lib/_http_server.js     |  9 +++++++--
 src/node.cc             | 13 +++++++++++++
 src/node_http_parser.cc |  6 ++++--
 7 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index c130f1a51dbc..8caa143bfbbc 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -131,6 +131,17 @@ Useful when activating the inspector by sending the `SIGUSR1` signal.
 Default host is 127.0.0.1.
 
 
+### `--insecure-http-parser`
+<!-- YAML
+added: REPLACEME
+-->
+
+Use an insecure HTTP parser that accepts invalid HTTP headers. This may allow
+interoperability with non-conformant HTTP implementations. It may also allow
+request smuggling and other HTTP attacks that rely on invalid headers being
+accepted. Avoid using this option.
+
+
 ### `--no-deprecation`
 <!-- YAML
 added: v0.8.0
@@ -476,6 +487,7 @@ Node.js options that are allowed are:
 - `--enable-fips`
 - `--force-fips`
 - `--icu-data-dir`
+- `--insecure-http-parser`
 - `--inspect-brk`
 - `--inspect-port`
 - `--inspect`
diff --git a/doc/node.1 b/doc/node.1
index 9447bacbe0a4..09a91c4b228b 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -110,6 +110,13 @@ Activate inspector on host:port and break at start of user script.
 .BR \-\-inspect-port \fI=[host:]port\fR
 Set the host:port to be used when the inspector is activated.
 
+.TP
+.BR \-\-insecure\-http\-parser
+Use an insecure HTTP parser that accepts invalid HTTP headers. This may allow
+interoperability with non-conformant HTTP implementations. It may also allow
+request smuggling and other HTTP attacks that rely on invalid headers being
+accepted. Avoid using this option.
+
 .TP
 .BR \-\-max\-http\-header-size \fI=size\fR
 Specify the maximum size of HTTP headers in bytes. Defaults to 8KB.
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 0dc8da0f4f6c..d0d6e8e3c3b7 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -31,7 +31,8 @@ const {
   debug,
   freeParser,
   httpSocketSetup,
-  parsers
+  parsers,
+  isLenient
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
 const Agent = require('_http_agent');
@@ -622,7 +623,9 @@ function tickOnSocket(req, socket) {
   var parser = parsers.alloc();
   req.socket = socket;
   req.connection = socket;
-  parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
+  parser.reinitialize(HTTPParser.RESPONSE,
+                      parser[is_reused_symbol],
+                      isLenient());
   if (process.domain) {
     process.domain.add(parser);
   }
diff --git a/lib/_http_common.js b/lib/_http_common.js
index a8086941cb6a..81ce7e1a30eb 100644
--- a/lib/_http_common.js
+++ b/lib/_http_common.js
@@ -353,6 +353,16 @@ function checkInvalidHeaderChar(val) {
   return false;
 }
 
+let warnedLenient = false;
+
+function isLenient() {
+  if (process.insecureHTTPParser && !warnedLenient) {
+    warnedLenient = true;
+    process.emitWarning('Using insecure HTTP parsing');
+  }
+  return !!process.insecureHTTPParser;
+}
+
 module.exports = {
   _checkInvalidHeaderChar: checkInvalidHeaderChar,
   _checkIsHttpToken: checkIsHttpToken,
@@ -364,5 +374,6 @@ module.exports = {
   httpSocketSetup,
   methods,
   parsers,
-  kIncomingMessage
+  kIncomingMessage,
+  isLenient
 };
diff --git a/lib/_http_server.js b/lib/_http_server.js
index ee5e19674865..5099070dfb03 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -34,7 +34,8 @@ const {
   chunkExpression,
   httpSocketSetup,
   kIncomingMessage,
-  _checkInvalidHeaderChar: checkInvalidHeaderChar
+  _checkInvalidHeaderChar: checkInvalidHeaderChar,
+  isLenient
 } = require('_http_common');
 const { OutgoingMessage } = require('_http_outgoing');
 const { outHeadersKey, ondrain, nowDate } = require('internal/http');
@@ -333,7 +334,11 @@ function connectionListenerInternal(server, socket) {
   socket.on('timeout', socketOnTimeout);
 
   var parser = parsers.alloc();
-  parser.reinitialize(HTTPParser.REQUEST, parser[is_reused_symbol]);
+  parser.reinitialize(
+    HTTPParser.REQUEST,
+    parser[is_reused_symbol],
+    isLenient()
+  );
   parser.socket = socket;
 
   // We are starting to wait for our headers.
diff --git a/src/node.cc b/src/node.cc
index 708a84554623..09c5954d6996 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -240,6 +240,9 @@ std::string icu_data_dir;  // NOLINT(runtime/string)
 
 uint64_t max_http_header_size = 8 * 1024;
 
+// Set in node.cc by ParseArgs when --insecure-http-parser is used.
+bool insecure_http_parser = false;
+
 // used by C++ modules as well
 bool no_deprecation = false;
 
@@ -3265,6 +3268,11 @@ void SetupProcessObject(Environment* env,
     preload_modules.clear();
   }
 
+  // --insecure-http-parser
+  if (insecure_http_parser) {
+    READONLY_PROPERTY(process, "insecureHTTPParser", True(env->isolate()));
+  }
+
   // --no-deprecation
   if (no_deprecation) {
     READONLY_PROPERTY(process, "noDeprecation", True(env->isolate()));
@@ -3539,6 +3547,8 @@ static void PrintHelp() {
          "  --inspect-port=[host:]port\n"
          "                             set host:port for inspector\n"
 #endif
+         "  --insecure-http-parser     use an insecure HTTP parser that\n"
+         "                             accepts invalid HTTP headers\n"
          "  --no-deprecation           silence deprecation warnings\n"
          "  --max-http-header-size     Specify the maximum size of HTTP\n"
          "                             headers in bytes. Defaults to 8KB.\n"
@@ -3686,6 +3696,7 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
   static const char* whitelist[] = {
     // Node options, sorted in `node --help` order for ease of comparison.
     "--require", "-r",
+    "--insecure-http-parser",
     "--inspect",
     "--inspect-brk",
     "--inspect-port",
@@ -3832,6 +3843,8 @@ static void ParseArgs(int* argc,
       syntax_check_only = true;
     } else if (strcmp(arg, "--interactive") == 0 || strcmp(arg, "-i") == 0) {
       force_repl = true;
+    } else if (strcmp(arg, "--insecure-http-parser") == 0) {
+      insecure_http_parser = true;
     } else if (strcmp(arg, "--no-deprecation") == 0) {
       no_deprecation = true;
     } else if (strcmp(arg, "--napi-modules") == 0) {
diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc
index dc59c243f69f..712976b75f4e 100644
--- a/src/node_http_parser.cc
+++ b/src/node_http_parser.cc
@@ -476,6 +476,7 @@ class Parser : public AsyncWrap {
 
   static void Reinitialize(const FunctionCallbackInfo<Value>& args) {
     Environment* env = Environment::GetCurrent(args);
+    bool lenient = args[2]->IsTrue();
 
     CHECK(args[0]->IsInt32());
     CHECK(args[1]->IsBoolean());
@@ -494,7 +495,7 @@ class Parser : public AsyncWrap {
     if (isReused) {
       parser->AsyncReset();
     }
-    parser->Init(type);
+    parser->Init(type, lenient);
   }
 
 
@@ -738,8 +739,9 @@ class Parser : public AsyncWrap {
   }
 
 
-  void Init(enum http_parser_type type) {
+  void Init(enum http_parser_type type, bool lenient) {
     http_parser_init(&parser_, type);
+    parser_.lenient_http_headers = lenient;
     url_.Reset();
     status_message_.Reset();
     num_fields_ = 0;