336 lines
11 KiB
Diff
336 lines
11 KiB
Diff
|
Ported from following patches,
|
|||
|
|
|||
|
|
|||
|
|
|||
|
http: disallow two-byte characters in URL path
|
|||
|
|
|||
|
CVE-2018-12116
|
|||
|
Backport of b961d9f to 6.x
|
|||
|
|
|||
|
Original commit:
|
|||
|
This commit changes node's handling of two-byte characters in
|
|||
|
the path component of an http URL. Previously, node would just
|
|||
|
strip the higher byte when generating the request. So this code:
|
|||
|
|
|||
|
```
|
|||
|
http.request({host: "example.com", port: "80", "/N"})
|
|||
|
```
|
|||
|
|
|||
|
would request `http://example.com/.`
|
|||
|
(`.` is the character for the byte `0x2e`).
|
|||
|
|
|||
|
This is not useful and can in some cases lead to filter evasion.
|
|||
|
With this change, the code generates `ERR_UNESCAPED_CHARACTERS`,
|
|||
|
just like space and control characters already did.
|
|||
|
|
|||
|
PR-URL: #16237
|
|||
|
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|||
|
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|||
|
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
|
|||
|
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
|||
|
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
|
|||
|
|
|||
|
PR-URL: nodejs-private/node-private#146
|
|||
|
Fixes: nodejs-private/security#207
|
|||
|
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>
|
|||
|
|
|||
|
|
|||
|
|
|||
|
|
|||
|
From 35344e87bfc7c01079502850ce023e781a85db2d Mon Sep 17 00:00:00 2001
|
|||
|
From: James M Snell <jasnell@gmail.com>
|
|||
|
Date: Wed, 16 Aug 2017 09:34:37 -0700
|
|||
|
Subject: [PATCH] src: minor cleanup for node_revert
|
|||
|
|
|||
|
Make the revert related functions inline to eliminate the need
|
|||
|
for node_revert.cc, prefitx the constants and the def, other misc
|
|||
|
cleanup
|
|||
|
|
|||
|
PR-URL: https://github.com/nodejs/node/pull/14864
|
|||
|
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|||
|
|
|||
|
|
|||
|
|
|||
|
|
|||
|
From dd20c0186fab9dd92b42f280b8d2b6f980f6b406 Mon Sep 17 00:00:00 2001
|
|||
|
From: Matteo Collina <hello@matteocollina.com>
|
|||
|
Date: Thu, 20 Sep 2018 15:02:26 +0200
|
|||
|
Subject: [PATCH] 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>
|
|||
|
|
|||
|
Index: node-v4.9.1/lib/_http_client.js
|
|||
|
===================================================================
|
|||
|
--- node-v4.9.1.orig/lib/_http_client.js
|
|||
|
+++ node-v4.9.1/lib/_http_client.js
|
|||
|
@@ -14,6 +14,10 @@ const OutgoingMessage = require('_http_o
|
|||
|
const Agent = require('_http_agent');
|
|||
|
const Buffer = require('buffer').Buffer;
|
|||
|
|
|||
|
+const REVERT_CVE_2018_12116 = process.REVERT_CVE_2018_12116;
|
|||
|
+
|
|||
|
+const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
|
|||
|
+
|
|||
|
|
|||
|
function ClientRequest(options, cb) {
|
|||
|
var self = this;
|
|||
|
@@ -40,15 +44,21 @@ function ClientRequest(options, cb) {
|
|||
|
if (self.agent && self.agent.protocol)
|
|||
|
expectedProtocol = self.agent.protocol;
|
|||
|
|
|||
|
- if (options.path && /[\u0000-\u0020]/.test(options.path)) {
|
|||
|
- // The actual regex 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. That's
|
|||
|
- // why it only scans for spaces because those are guaranteed to create
|
|||
|
- // an invalid request.
|
|||
|
- throw new TypeError('Request path contains unescaped characters.');
|
|||
|
- } else if (protocol !== expectedProtocol) {
|
|||
|
+ var path;
|
|||
|
+ if (options.path) {
|
|||
|
+ path = String(options.path);
|
|||
|
+ var invalidPath;
|
|||
|
+ if (REVERT_CVE_2018_12116) {
|
|||
|
+ invalidPath = /[\u0000-\u0020]/.test(path);
|
|||
|
+ } else {
|
|||
|
+ invalidPath = INVALID_PATH_REGEX.test(path);
|
|||
|
+ }
|
|||
|
+
|
|||
|
+ if (invalidPath)
|
|||
|
+ throw new TypeError('Request path contains unescaped characters');
|
|||
|
+ }
|
|||
|
+
|
|||
|
+ if (protocol !== expectedProtocol) {
|
|||
|
throw new Error('Protocol "' + protocol + '" not supported. ' +
|
|||
|
'Expected "' + expectedProtocol + '".');
|
|||
|
}
|
|||
|
Index: node-v4.9.1/test/parallel/test-http-client-invalid-path.js
|
|||
|
===================================================================
|
|||
|
--- /dev/null
|
|||
|
+++ node-v4.9.1/test/parallel/test-http-client-invalid-path.js
|
|||
|
@@ -0,0 +1,13 @@
|
|||
|
+'use strict';
|
|||
|
+
|
|||
|
+require('../common');
|
|||
|
+
|
|||
|
+const http = require('http');
|
|||
|
+const assert = require('assert');
|
|||
|
+
|
|||
|
+assert.throws(() => {
|
|||
|
+ http.request({
|
|||
|
+ path: '/thisisinvalid\uffe2'
|
|||
|
+ }).end();
|
|||
|
+}, /Request path contains unescaped characters/);
|
|||
|
+
|
|||
|
Index: node-v4.9.1/node.gyp
|
|||
|
===================================================================
|
|||
|
--- node-v4.9.1.orig/node.gyp
|
|||
|
+++ node-v4.9.1/node.gyp
|
|||
|
@@ -152,7 +152,6 @@
|
|||
|
'src/node_javascript.cc',
|
|||
|
'src/node_main.cc',
|
|||
|
'src/node_os.cc',
|
|||
|
- 'src/node_revert.cc',
|
|||
|
'src/node_util.cc',
|
|||
|
'src/node_v8.cc',
|
|||
|
'src/node_stat_watcher.cc',
|
|||
|
Index: node-v4.9.1/src/node.cc
|
|||
|
===================================================================
|
|||
|
--- node-v4.9.1.orig/src/node.cc
|
|||
|
+++ node-v4.9.1/src/node.cc
|
|||
|
@@ -153,6 +153,9 @@ static node_module* modlist_builtin;
|
|||
|
static node_module* modlist_linked;
|
|||
|
static node_module* modlist_addon;
|
|||
|
|
|||
|
+// Bit flag used to track security reverts (see node_revert.h)
|
|||
|
+unsigned int reverted = 0;
|
|||
|
+
|
|||
|
#if defined(NODE_HAVE_I18N_SUPPORT)
|
|||
|
// Path to ICU data (for i18n / Intl)
|
|||
|
static const char* icu_data_dir = nullptr;
|
|||
|
@@ -3126,11 +3129,11 @@ void SetupProcessObject(Environment* env
|
|||
|
// --security-revert flags
|
|||
|
#define V(code, _, __) \
|
|||
|
do { \
|
|||
|
- if (IsReverted(REVERT_ ## code)) { \
|
|||
|
+ if (IsReverted(SECURITY_REVERT_ ## code)) { \
|
|||
|
READONLY_PROPERTY(process, "REVERT_" #code, True(env->isolate())); \
|
|||
|
} \
|
|||
|
} while (0);
|
|||
|
- REVERSIONS(V)
|
|||
|
+ SECURITY_REVERSIONS(V)
|
|||
|
#undef V
|
|||
|
|
|||
|
size_t exec_path_len = 2 * PATH_MAX;
|
|||
|
Index: node-v4.9.1/src/node_revert.cc
|
|||
|
===================================================================
|
|||
|
--- node-v4.9.1.orig/src/node_revert.cc
|
|||
|
+++ /dev/null
|
|||
|
@@ -1,53 +0,0 @@
|
|||
|
-#include "node_revert.h"
|
|||
|
-#include <stdio.h>
|
|||
|
-#include <string.h>
|
|||
|
-
|
|||
|
-namespace node {
|
|||
|
-
|
|||
|
-unsigned int reverted = 0;
|
|||
|
-
|
|||
|
-const char* RevertMessage(const unsigned int cve) {
|
|||
|
-#define V(code, label, msg) case REVERT_ ## code: return label ": " msg;
|
|||
|
- switch (cve) {
|
|||
|
- REVERSIONS(V)
|
|||
|
- default:
|
|||
|
- return "Unknown";
|
|||
|
- }
|
|||
|
-#undef V
|
|||
|
-}
|
|||
|
-
|
|||
|
-void Revert(const unsigned int cve) {
|
|||
|
- reverted |= 1 << cve;
|
|||
|
- printf("SECURITY WARNING: Reverting %s\n", RevertMessage(cve));
|
|||
|
-}
|
|||
|
-
|
|||
|
-void Revert(const char* cve) {
|
|||
|
-#define V(code, label, _) \
|
|||
|
- do { \
|
|||
|
- if (strcmp(cve, label) == 0) { \
|
|||
|
- Revert(static_cast<unsigned int>(REVERT_ ## code)); \
|
|||
|
- return; \
|
|||
|
- } \
|
|||
|
- } while (0);
|
|||
|
- REVERSIONS(V)
|
|||
|
-#undef V
|
|||
|
- printf("Error: Attempt to revert an unknown CVE [%s]\n", cve);
|
|||
|
- exit(12);
|
|||
|
-}
|
|||
|
-
|
|||
|
-bool IsReverted(const unsigned int cve) {
|
|||
|
- return reverted & (1 << cve);
|
|||
|
-}
|
|||
|
-
|
|||
|
-bool IsReverted(const char * cve) {
|
|||
|
-#define V(code, label, _) \
|
|||
|
- do { \
|
|||
|
- if (strcmp(cve, label) == 0) \
|
|||
|
- return IsReverted(static_cast<unsigned int>(REVERT_ ## code)); \
|
|||
|
- } while (0);
|
|||
|
- REVERSIONS(V)
|
|||
|
- return false;
|
|||
|
-#undef V
|
|||
|
-}
|
|||
|
-
|
|||
|
-} // namespace node
|
|||
|
Index: node-v4.9.1/src/node_revert.h
|
|||
|
===================================================================
|
|||
|
--- node-v4.9.1.orig/src/node_revert.h
|
|||
|
+++ node-v4.9.1/src/node_revert.h
|
|||
|
@@ -2,6 +2,7 @@
|
|||
|
#define SRC_NODE_REVERT_H_
|
|||
|
|
|||
|
#include "node.h"
|
|||
|
+#include <string.h>
|
|||
|
|
|||
|
/**
|
|||
|
* Note that it is expected for this list to vary across specific LTS and
|
|||
|
@@ -10,33 +11,56 @@
|
|||
|
* consensus.
|
|||
|
*
|
|||
|
* For *master* this list should always be empty!
|
|||
|
- *
|
|||
|
**/
|
|||
|
-#define REVERSIONS(XX) \
|
|||
|
- XX(CVE_2016_2216, "CVE-2016-2216", "Strict HTTP Header Parsing")
|
|||
|
-
|
|||
|
namespace node {
|
|||
|
|
|||
|
-typedef enum {
|
|||
|
-#define V(code, _, __) REVERT_ ## code,
|
|||
|
- REVERSIONS(V)
|
|||
|
+#define SECURITY_REVERSIONS(XX) \
|
|||
|
+ XX(CVE_2016_2216, "CVE-2016-2216", "Strict HTTP Header Parsing") \
|
|||
|
+ XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting")
|
|||
|
+
|
|||
|
+enum reversion {
|
|||
|
+#define V(code, ...) SECURITY_REVERT_##code,
|
|||
|
+ SECURITY_REVERSIONS(V)
|
|||
|
#undef V
|
|||
|
-} reversions_t;
|
|||
|
+};
|
|||
|
|
|||
|
-/* A bit field for tracking the active reverts */
|
|||
|
extern unsigned int reverted;
|
|||
|
|
|||
|
-/* Revert the given CVE (see reversions_t enum) */
|
|||
|
-void Revert(const unsigned int cve);
|
|||
|
-
|
|||
|
-/* Revert the given CVE by label */
|
|||
|
-void Revert(const char* cve);
|
|||
|
-
|
|||
|
-/* true if the CVE has been reverted **/
|
|||
|
-bool IsReverted(const unsigned int cve);
|
|||
|
+inline const char* RevertMessage(const reversion cve) {
|
|||
|
+#define V(code, label, msg) case SECURITY_REVERT_##code: return label ": " msg;
|
|||
|
+ switch (cve) {
|
|||
|
+ SECURITY_REVERSIONS(V)
|
|||
|
+ default:
|
|||
|
+ return "Unknown";
|
|||
|
+ }
|
|||
|
+#undef V
|
|||
|
+}
|
|||
|
|
|||
|
-/* true if the CVE has been reverted **/
|
|||
|
-bool IsReverted(const char * cve);
|
|||
|
+inline void Revert(const reversion cve) {
|
|||
|
+ reverted |= 1 << cve;
|
|||
|
+ printf("SECURITY WARNING: Reverting %s\n", RevertMessage(cve));
|
|||
|
+}
|
|||
|
+
|
|||
|
+inline void Revert(const char* cve) {
|
|||
|
+#define V(code, label, _) \
|
|||
|
+ if (strcmp(cve, label) == 0) return Revert(SECURITY_REVERT_##code);
|
|||
|
+ SECURITY_REVERSIONS(V)
|
|||
|
+#undef V
|
|||
|
+ printf("Error: Attempt to revert an unknown CVE [%s]\n", cve);
|
|||
|
+ exit(12);
|
|||
|
+}
|
|||
|
+
|
|||
|
+inline bool IsReverted(const reversion cve) {
|
|||
|
+ return reverted & (1 << cve);
|
|||
|
+}
|
|||
|
+
|
|||
|
+inline bool IsReverted(const char* cve) {
|
|||
|
+#define V(code, label, _) \
|
|||
|
+ if (strcmp(cve, label) == 0) return IsReverted(SECURITY_REVERT_##code);
|
|||
|
+ SECURITY_REVERSIONS(V)
|
|||
|
+ return false;
|
|||
|
+#undef V
|
|||
|
+}
|
|||
|
|
|||
|
} // namespace node
|
|||
|
|
|||
|
Index: node-v4.9.1/src/node_http_parser.cc
|
|||
|
===================================================================
|
|||
|
--- node-v4.9.1.orig/src/node_http_parser.cc
|
|||
|
+++ node-v4.9.1/src/node_http_parser.cc
|
|||
|
@@ -679,7 +679,7 @@ class Parser : public AsyncWrap {
|
|||
|
void Init(enum http_parser_type type) {
|
|||
|
http_parser_init(&parser_, type);
|
|||
|
/* Allow the strict http header parsing to be reverted */
|
|||
|
- parser_.lenient_http_headers = IsReverted(REVERT_CVE_2016_2216) ? 1 : 0;
|
|||
|
+ parser_.lenient_http_headers = IsReverted(SECURITY_REVERT_CVE_2016_2216) ? 1 : 0;
|
|||
|
url_.Reset();
|
|||
|
status_message_.Reset();
|
|||
|
num_fields_ = 0;
|