http: add --security-revert for CVE-2018-12116

PR-URL: https://github.com/nodejs-private/node-private/pull/146
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

2 files changed, 44 insertions, 3 deletions

diff --git a/lib/_http_client.js b/lib/_http_client.js
index 20d05efb5ec..7b5798fe021 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -41,6 +41,36 @@ const { outHeadersKey, ondrain } = require('internal/http');
 const { nextTick } = require('internal/process/next_tick');
 const is_reused_symbol = require('internal/freelist').symbols.is_reused_symbol;
 
+const REVERT_CVE_2018_12116 = process.REVERT_CVE_2018_12116;
+
+// DO NOT USE: this is insecure. See CVE-2018-12116.
+// The actual list of disallowed characters in regexp form is more like:
+//    /[^A-Za-z0-9\-._~!$&'()*+,;=/:@]/
+// with an additional rule for ignoring percentage-escaped characters, but
+// that's a) hard to capture in a regular expression that performs well, and
+// b) possibly too restrictive for real-world usage. So instead we restrict the
+// filter to just control characters and spaces.
+//
+// This function is used in the case of small paths, where manual character code
+// checks can greatly outperform the equivalent regexp (tested in V8 5.4).
+function isInvalidPath(s) {
+  var i = 0;
+  if (s.charCodeAt(0) <= 32) return true;
+  if (++i >= s.length) return false;
+  if (s.charCodeAt(1) <= 32) return true;
+  if (++i >= s.length) return false;
+  if (s.charCodeAt(2) <= 32) return true;
+  if (++i >= s.length) return false;
+  if (s.charCodeAt(3) <= 32) return true;
+  if (++i >= s.length) return false;
+  if (s.charCodeAt(4) <= 32) return true;
+  if (++i >= s.length) return false;
+  if (s.charCodeAt(5) <= 32) return true;
+  ++i;
+  for (; i < s.length; ++i)
+    if (s.charCodeAt(i) <= 32) return true;
+  return false;
+}
 const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
 
 function validateHost(host, name) {
@@ -92,7 +122,18 @@ function ClientRequest(options, cb) {
   var path;
   if (options.path) {
     path = String(options.path);
-    if (INVALID_PATH_REGEX.test(path))
+    var invalidPath;
+    if (REVERT_CVE_2018_12116) {
+      if (path.length <= 39) { // Determined experimentally in V8 5.4
+        invalidPath = isInvalidPath(path);
+      } else {
+        invalidPath = /[\u0000-\u0020]/.test(path);
+      }
+    } else {
+      invalidPath = INVALID_PATH_REGEX.test(path);
+    }
+
+    if (invalidPath)
       throw new TypeError('Request path contains unescaped characters');
   }
 
diff --git a/src/node_revert.h b/src/node_revert.h
index c5963afeafd..9b4f1a9bbf6 100644
--- a/src/node_revert.h
+++ b/src/node_revert.h
@@ -15,8 +15,8 @@
  **/
 namespace node {
 
-#define SECURITY_REVERSIONS(XX)
-//  XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
+#define SECURITY_REVERSIONS(XX) \
+  XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting")
 
 enum reversion {
 #define V(code, ...) SECURITY_REVERT_##code,