path: fix regression in posix.normalize by targos · Pull Request #19520 · nodejs/node

targos · 2018-03-21T18:25:15Z

Fixes a regression introduced in
4ae320f
The posix version of normalize should not treat backslash as a path
separator.

Fixes: #19519

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Fixes a regression introduced in nodejs@4ae320f The posix version of normalize should not treat backslash as a path separator. Fixes: nodejs#19519

mscdex · 2018-03-21T21:05:10Z

lib/path.js

@@ -364,7 +369,8 @@ const win32 = {

    var tail;
    if (rootEnd < len)


Can braces be added for the if and else here if we start having multi-line conditional bodies?

targos · 2018-03-22T07:16:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/13821/

MylesBorins · 2018-03-22T10:46:31Z

Should we fast track this patch / a 9.9.1 or can this wait for next week?

targos · 2018-03-22T15:01:18Z

It's difficult to estimate the impact (I suppose backslashes are not common in unixes paths). It can probably wait and land with the next security release.

targos · 2018-03-23T18:48:44Z

Landed in a0adf56

Fixes a regression introduced in [1]. The posix version of normalize should not treat backslash as a path separator. [1] 4ae320f2 PR-URL: #19520 Fixes: #19519 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

SimenB · 2018-03-26T10:45:38Z

Any plans for a patch with this?

targos · 2018-03-26T13:21:56Z

This is ready to land in the next patch release, which should happen tomorrow: https://nodejs.org/en/blog/vulnerability/march-2018-security-releases/

Notable changes: * cluster: - Add support for `NODE_OPTIONS="--inspect"` (Sameer Srivastava) #19165 * crypto: - Expose the public key of a certificate (Hannes Magnusson) #17690 * n-api: - Add `napi_fatal_exception` to trigger an `uncaughtException` in JavaScript (Mathias Buus) #19337 * path: - Fix regression in `posix.normalize` (Michaël Zasso) #19520 * stream: - Improve stream creation performance (Brian White) #19401 * Added new collaborators - [BethGriggs](https://github.com/BethGriggs) Beth Griggs

This is a security release. All Node.js users should consult the security release summary at: https://nodejs.org/en/blog/vulnerability/march-2018-security-releases/ for details on patched vulnerabilities. Fixes for the following CVEs are included in this release: * CVE-2018-7158 * CVE-2018-7159 * CVE-2018-7160 Notable changes: * Upgrade to OpenSSL 1.0.2o: Does not contain any security fixes that are known to impact Node.js. * **Fix for inspector DNS rebinding vulnerability (CVE-2018-7160)**: A malicious website could use a DNS rebinding attack to trick a web browser to bypass same-origin-policy checks and allow HTTP connections to localhost or to hosts on the local network, potentially to an open inspector port as a debugger, therefore gaining full code execution access. The inspector now only allows connections that have a browser `Host` value of `localhost` or `localhost6`. * **Fix for `'path'` module regular expression denial of service (CVE-2018-7158)**: A regular expression used for parsing POSIX an Windows paths could be used to cause a denial of service if an attacker were able to have a specially crafted path string passed through one of the impacted `'path'` module functions. * **Reject spaces in HTTP `Content-Length` header values (CVE-2018-7159)**: The Node.js HTTP parser allowed for spaces inside `Content-Length` header values. Such values now lead to rejected connections in the same way as non-numeric values. * **Update root certificates**: 5 additional root certificates have been added to the Node.js binary and 30 have been removed. * cluster: - Add support for `NODE_OPTIONS="--inspect"` (Sameer Srivastava) #19165 * crypto: - Expose the public key of a certificate (Hannes Magnusson) #17690 * n-api: - Add `napi_fatal_exception` to trigger an `uncaughtException` in JavaScript (Mathias Buus) #19337 * path: - Fix regression in `posix.normalize` (Michaël Zasso) #19520 * stream: - Improve stream creation performance (Brian White) #19401 * Added new collaborators - [BethGriggs](https://github.com/BethGriggs) Beth Griggs PR-URL: https://github.com/nodejs-private/node-private/pull/111

This is a security release. All Node.js users should consult the security release summary at: https://nodejs.org/en/blog/vulnerability/march-2018-security-releases/ for details on patched vulnerabilities. Fixes for the following CVEs are included in this release: * CVE-2018-7158 * CVE-2018-7159 * CVE-2018-7160 Notable changes: * Upgrade to OpenSSL 1.0.2o: Does not contain any security fixes that are known to impact Node.js. * **Fix for inspector DNS rebinding vulnerability (CVE-2018-7160)**: A malicious website could use a DNS rebinding attack to trick a web browser to bypass same-origin-policy checks and allow HTTP connections to localhost or to hosts on the local network, potentially to an open inspector port as a debugger, therefore gaining full code execution access. The inspector now only allows connections that have a browser `Host` value of `localhost` or `localhost6`. * **Fix for `'path'` module regular expression denial of service (CVE-2018-7158)**: A regular expression used for parsing POSIX an Windows paths could be used to cause a denial of service if an attacker were able to have a specially crafted path string passed through one of the impacted `'path'` module functions. * **Reject spaces in HTTP `Content-Length` header values (CVE-2018-7159)**: The Node.js HTTP parser allowed for spaces inside `Content-Length` header values. Such values now lead to rejected connections in the same way as non-numeric values. * **Update root certificates**: 5 additional root certificates have been added to the Node.js binary and 30 have been removed. * cluster: - Add support for `NODE_OPTIONS="--inspect"` (Sameer Srivastava) nodejs/node#19165 * crypto: - Expose the public key of a certificate (Hannes Magnusson) nodejs/node#17690 * n-api: - Add `napi_fatal_exception` to trigger an `uncaughtException` in JavaScript (Mathias Buus) nodejs/node#19337 * path: - Fix regression in `posix.normalize` (Michaël Zasso) nodejs/node#19520 * stream: - Improve stream creation performance (Brian White) nodejs/node#19401 * Added new collaborators - [BethGriggs](https://github.com/BethGriggs) Beth Griggs PR-URL: https://github.com/nodejs-private/node-private/pull/111

path: fix regression in posix.normalize

3b978c6

Fixes a regression introduced in nodejs@4ae320f The posix version of normalize should not treat backslash as a path separator. Fixes: nodejs#19519

nodejs-github-bot added the path Issues and PRs related to the path subsystem. label Mar 21, 2018

targos mentioned this pull request Mar 21, 2018

Breaking change in path normalizeString in node 9.9.0 #19519

Closed

Trott approved these changes Mar 21, 2018

View reviewed changes

tniessen approved these changes Mar 21, 2018

View reviewed changes

mscdex reviewed Mar 21, 2018

View reviewed changes

cjihrig approved these changes Mar 21, 2018

View reviewed changes

braces

579847d

targos mentioned this pull request Mar 22, 2018

path: remove redundant function #19237

Closed

4 tasks

jasnell approved these changes Mar 23, 2018

View reviewed changes

targos closed this Mar 23, 2018

targos deleted the fix-path-normalize-posix branch March 23, 2018 18:48

astorije mentioned this pull request Mar 25, 2018

Exclude Node v9.9 from Travis build, for now thelounge/thelounge#2296

Merged

simonkberg mentioned this pull request Mar 26, 2018

Node 9.9.0 breaks test suite of Jest #19607

Closed

SimenB mentioned this pull request Mar 26, 2018

Failing test in node 9.9.0 jestjs/jest#5871

Closed

thymikee mentioned this pull request Mar 26, 2018

Expose SHA-1s via HasteFS jestjs/jest#5874

Merged

BethGriggs added the dont-land-on-v8.x label Oct 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

path: fix regression in posix.normalize#19520

path: fix regression in posix.normalize#19520
targos wants to merge 2 commits intonodejs:masterfrom
targos:fix-path-normalize-posix

targos commented Mar 21, 2018

Uh oh!

mscdex Mar 21, 2018

Uh oh!

targos commented Mar 22, 2018

Uh oh!

MylesBorins commented Mar 22, 2018

Uh oh!

targos commented Mar 22, 2018

Uh oh!

targos commented Mar 23, 2018

Uh oh!

SimenB commented Mar 26, 2018

Uh oh!

targos commented Mar 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

		@@ -364,7 +369,8 @@ const win32 = {

		var tail;
		if (rootEnd < len)

Uh oh!

Conversation

targos commented Mar 21, 2018

Checklist

Uh oh!

mscdex Mar 21, 2018

Choose a reason for hiding this comment

Uh oh!

targos commented Mar 22, 2018

Uh oh!

MylesBorins commented Mar 22, 2018

Uh oh!

targos commented Mar 22, 2018

Uh oh!

targos commented Mar 23, 2018

Uh oh!

SimenB commented Mar 26, 2018

Uh oh!

targos commented Mar 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants