Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
http: add --security-revert for CVE-2018-12116
PR-URL: nodejs-private/node-private#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>
  • Loading branch information
mcollina authored and rvagg committed Nov 27, 2018
1 parent 513e974 commit 576038f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
43 changes: 42 additions & 1 deletion lib/_http_client.js
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {

This comment has been minimized.

Copy link
@MaSven

MaSven Sep 20, 2023

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');
}

Expand Down
4 changes: 2 additions & 2 deletions src/node_revert.h
Expand Up @@ -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,
Expand Down

0 comments on commit 576038f

Please sign in to comment.