Skip to content

Commit

Permalink
http: disallow two-byte characters in URL path
Browse files Browse the repository at this point in the history
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 <[email protected]>
  Reviewed-By: Anna Henningsen <[email protected]>
  Reviewed-By: Anatoli Papirovski <[email protected]>
  Reviewed-By: Ruben Bridgewater <[email protected]>
  Reviewed-By: Timothy Gu <[email protected]>

PR-URL: nodejs-private/node-private#146
Fixes: nodejs-private/security#207
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sakthipriyan Vairamani <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Timothy Gu <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
bennofs authored and rvagg committed Nov 27, 2018
1 parent 9c268d0 commit 811b63c
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 8 deletions.
17 changes: 9 additions & 8 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const OutgoingMessage = require('_http_outgoing').OutgoingMessage;
const Agent = require('_http_agent');
const Buffer = require('buffer').Buffer;

const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;

function ClientRequest(options, cb) {
var self = this;
Expand Down Expand Up @@ -43,14 +44,14 @@ 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.
// Restrict the filter to control characters and spaces.
throw new TypeError('Request path contains unescaped characters');
} else if (protocol !== expectedProtocol) {
var path;
if (options.path) {
path = String(options.path);
if (INVALID_PATH_REGEX.test(path))
throw new TypeError('Request path contains unescaped characters');
}

if (protocol !== expectedProtocol) {
throw new Error('Protocol "' + protocol + '" not supported. ' +
'Expected "' + expectedProtocol + '"');
}
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-http-client-invalid-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'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/);

0 comments on commit 811b63c

Please sign in to comment.