node_modules parent climbing is a security risk

[ivan]

node climbs up to find modules, even in places like /home/node_modules, /node_modules, or C:\node_modules on Windows. The Windows case is particularly problematic because any user can create a directory in C:\.

This has been brought up before:
https://groups.google.com/forum/#!searchin/nodejs/node_modules$20security/nodejs/5BGr5dliUIk/abJEH3sPymcJ

Whether you like the behavior or not, node is compromising the security of multi-user servers and desktops. A shared hosting provider shouldn't need to know that they need to blacklist a "node_modules" username, and developers on Windows shouldn't need to create and secure a C:\node_modules directory before developing or running node.js software.

Here's a straw man proposal: by default, blacklist certain paths: /node_modules and C:\node_modules. If running inside $HOME, don't climb up outside $HOME (this effectively blacklists /home/node_modules and C:\Users\node_modules). Also, avoid loading from directories that appear to allow anyone to write to them. For the few exotic deployments out there, expose node's original search behavior behind an argument.

[iankronquist]

I'm taking a look at this.

[robotlolita]

I think not searching above the project's package.json, the cwd, or some other way to define the project root (maybe an additional command line option --project-root=<Path>) could be a reasonable option. Might break existing applications though.

[chrisdickinson]

This appears to have gotten a soft -1 from isaac when it came up on the mailing list at the time. To recap that argument here: a calamity of security flaws have to align to allow a user to compromise your node modules in this fashion:

1. They have to have access to your box.
2. They have to have write access to a dir adjacent to one of your working directories. (Exemplified here by "create a user named 'node_modules'" approach).
3. Your code must require a module that would not exist in their current working directory, or any of the ascending working directories -- in essence, your application would likely have to be broken to trigger this sort of behavior.

[ivan]

#2 is almost always true on multi-user Windows systems, and #3 would apply to any node.js software that has a dependency that does try { require('optional-module') } catch(e) {} where optional-module wasn't installed. And even if that weren't the case, can you imagine explaining "never require anything that doesn't exist in your own parent directories" as a necessary security measure? An attacker can easily stuff C:\node_modules with the filenames of every npm module, so one slip-up would mean you're toast.

[davepacheco]

I like @robotlolita's suggestion to allow users to specify an optional root. It's a whitelist rather than a blacklist, and it can be enforced absolutely if desired. You could either have it avoid opening any files outside that tree or just indicate that the module search should terminate there (in which case symlinks to other trees would still work, which is useful). I think it would be better to avoid assuming certain policy decisions like blacklisting /node_modules or stopping traversal at $HOME.

As others have pointed out, this is not a bug, since it's working as designed and documented[0]. The resulting behavior may make Node today unsuitable for certain environments (and in ways that are not obvious), and I think it makes sense to add a feature to improve that, but I only see this affecting Windows deployments and hosting environments that use a shared, non-isolated filesystem[1]. Both are sad (and shared hosting is sad for security for other reasons), but I don't think it's reasonable to call this a Node security issue any more than it's a security issue that you can specify arbitrary programs on the command line. The functionality in question is just too general-purpose and requires a surrounding system that's configured somewhat dangerously in order to abuse it.

[0] http://nodejs.org/api/modules.html#modules_loading_from_node_modules_folders

[1] On a Unix system, the attacker needs either write access to a parent directory of the victim's application (in which case the victim is completely compromised anyway) or needs to trick an operator into creating a "node_modules" directory in one of those parent directories. The only way I can see to do the latter is creating a home directory.

[tjfontaine]

There are definitely ways we can improve sandboxing in the future for node.

In the meantime you can explore existing mechanisms users are using for locking down the paths that are allowed for require with modules like: https://www.npmjs.com/package/fs-lock

[chrisdickinson]

It's a bit late for this now, but: in the future, if you suspect you have found a security issue, the appropriate venue for responsible disclosure is to email [email protected] with a description of the exploit -- it should not go through github issues.

[bmeck]

I think if we are on a multi-user machine this is something that should be locked down already by the system admin, and creating more complexity to the module loader is... a bad idea. Per process settings are pretty easy to reset in various ways (env vars can be reset, spawning child processes can get around this).

Possible solutions:

• *Configure the Windows FS if you are using multiple users on the same machine *
  • *do this anyway *
• Env var chroot
  • susceptible to env change if process is exploitable
• Arg flag chroot
  • susceptible if a child process is spawned w/o flag
• Hard coded
  • Limited and non-configurable
• Warn if there is a node_modules folder at root
  • only a warning, not an abort

Scenario Research:

Note: Group:R means by default all Users in a group can still potentially read in directories made here even if the folder itself has permissions revoked after creating modules if the Group has Bypass Traversal ( http://technet.microsoft.com/en-us/library/dn221950%28v=ws.10%29.aspx [Security Considerations] + http://msdn.microsoft.com/en-us/library/windows/desktop/aa363858%28v=vs.85%29.aspx [default ACL] ). The Everyone group has this by default.

XP (Semi-realistic scenario):

• Folder Creation: Users
• Bypass Traverse exploit: Everyone

C:\>C:\WINDOWS\system32\cacls C:\
C:\ BUILTIN\Administrators:(OI)(CI)F
    NT AUTHORITY\SYSTEM:(OI)(CI)F
    CREATOR OWNER:(OI)(CI)(IO)F
    BUILTIN\Users:(OI)(CI)R
    BUILTIN\Users:(CI)(special access:)
                      FILE_APPEND_DATA

    BUILTIN\Users:(CI)(IO)(special access:)
                          FILE_WRITE_DATA

    Everyone:R

Vista (User creation at fault / Lack of dropping privileges at fault):

• Folder Creation: Authenticated Users (Users !Guest !Anonymous)
• Bypass Traverse exploit: None

C:\>C:\Windows\System32\cacls C:\
C:\ BUILTIN\Administrators:F
    BUILTIN\Administrators:(OI)(CI)(IO)F
    NT AUTHORITY\SYSTEM:F
    NT AUTHORITY\SYSTEM:(OI)(CI)(IO)F
    BUILTIN\Users:(OI)(CI)R
    NT AUTHORITY\Authenticated Users:(OI)(CI)(IO)C
    NT AUTHORITY\Authenticated Users:(special access:)
                                     FILE_APPEND_DATA

7 (User creation at fault / Lack of dropping privileges at fault):

• Folder Creation: Authenticated Users (Users !Guest !Anonymous)
• Bypass Traverse exploit: None

C:\>C:\Windows\System32\cacls C:\
C:\ BUILTIN\Administrators:F
    BUILTIN\Administrators:(OI)(CI)(IO)F
    NT AUTHORITY\SYSTEM:F
    NT AUTHORITY\SYSTEM:(OI)(CI)(IO)F
    BUILTIN\Users:(OI)(CI)R
    NT AUTHORITY\Authenticated Users:(OI)(CI)(IO)C
    NT AUTHORITY\Authenticated Users:(special access:)
                                     FILE_APPEND_DATA

8 (User creation at fault / Lack of dropping privileges at fault):

• Folder Creation: Authenticated Users (Users !Guest !Anonymous)
• Bypass Traverse exploit: None

C:\>C:\Windows\System32\cacls C:\
C:\ BUILTIN\Administrators:(OI)(CI)F
    NT AUTHORITY\SYSTEM:(OI)(CI)F
    BUILTIN\Users:(OI)(CI)R
    NT AUTHORITY\Authenticated Users:(OI)(CI)(IO)C
    NT AUTHORITY\Authenticated Users:(special access:)
                                     FILE_APPEND_DATA

Conclusion

In Windows XP there is a somewhat realistic scenario where this could occur if someone got guest/anonymous access to a machine. This exploit is limited to created/authenticated users after that.

Recommendation

Users should not be able to write to drive root paths by default anyway if you are administering a multi-user machine. Do this regardless of the node issue.

XP: Immediate removal of Everyone group from roots of drives upon creation

Vista - 8: Remove permissions for relevant directories where Users should not be allowed to create folders prior to User creation.

[jasnell]

@joyent/node-coreteam ... any further thoughts on this one? I'm personally leaning towards closing without further action.