Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncontrolled search path element in Node.js on Windows #34124

Closed
l01cd3v opened this issue Jun 29, 2020 · 3 comments
Closed

Uncontrolled search path element in Node.js on Windows #34124

l01cd3v opened this issue Jun 29, 2020 · 3 comments
Labels
module Issues and PRs related to the module subsystem. security Issues and PRs related to security.

Comments

@l01cd3v
Copy link

l01cd3v commented Jun 29, 2020

  • Version: all
  • Platform: Windows

The node.js load module functionality described at https://nodejs.org/api/modules.html#modules_loading_from_node_modules_folders is essentially an uncontrolled search path element (CWE-427) vulnerability that enables local and/or horizontal privilege escalation. In particular, on a Windows system, Node.js will attempt to load missing modules from a number of locations, including C:\node_modules\ which is a location that can be written-to by any user by default.

We understand that this is a known issue that has been discussed previously (e.g. here back in 2014: nodejs/node-v0.x-archive#8830) but believe that it still presents a significant security risk for certain classes of applications. The vast majority of operating systems have a threat model where all standard users are not considered equal, with access controls in place to prevent two low-privilege users from accessing each other’s data. It appears that the threat profile used by Node.js does not take this in account, which means that the principle of least privilege cannot be properly applied on a multi-user operating system running a node application.

Our experience indicates that missing modules are more often caused by a try { require ‘missing-module’; } catch () statement rather than a malformed package file. In practice, this behavior is all-too-common and the complexity of the dependency tree for even a simple Node application makes detecting all instances where this search path is triggered an intractable problem.

@addaleax addaleax added module Issues and PRs related to the module subsystem. security Issues and PRs related to security. labels Jun 29, 2020
@bzoz
Copy link
Contributor

bzoz commented Jul 1, 2020

The linked discussion suggests using a parameter or env variable for "project root" above which Node would not search for the module. The env variable sounds like a reasonable solution. Maybe a NODE_ALLOWED_MODULE_LOCATIONS with a semicolon-separated list of folders that Node will only load modules from?

bzoz added a commit to JaneaSystems/node that referenced this issue Jul 15, 2020
Adds --allowed-module-location option that can be used to limit the
paths Node.js is allowed to load modules from.

Fixes: nodejs#34124
@judy-fei
Copy link

it seems --allowed-module-location has been removed, right? because i cannot search this flag in the latest nodejs, and i also try to add env variable NODE_ALLOWED_MODULE_LOCATIONS, but it seems not work for this issue, node still search all node_modules for parent folders.

@bnoordhuis
Copy link
Member

@judy-fei It looks like you already received an answer in the linked (closed & unmerged) pull request. I'll go ahead and close this as a wontfix. For anyone coming here from search engines:

  • node's behavior was, is and continues to be to search upwards the directory tree
  • use policies (see discussion in PR) if you want to stop it from searching all the way to the root

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem. security Issues and PRs related to security.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants