Opening files in Node.js considered harmful
TLDR: https://github.com/joyent/node/issues/6905, fixed in libuv v0.11 branch
I found a bug that I thought was interesting since it is in a ‘mature’ platform (Node.js) and a potential major security vulnerability and resource leak.
At my job, we written an agent component that consists of a Node.js collection ‘coordinator’, and some native C ‘workers’ that are spawned as child processes. We were troubleshooting some performance issues with one of our C workers, when one of our engineers (Mark — aka @strcpy, our resident C hacker and all-around systems badass) pointed out to me that it had open file descriptors to files that the code should never have opened.
Jen, why is your node shit leaking file descriptors into my app? **yells and throws things around like an ape that’s been given a cement banana**
Mark discovered this using the unix tool `lsof` (which, as it name implies, ‘lists open files’). Imagine ‘native_binary’ is our native child process, and ‘node’ is the parent process that spawned it:
# both parent and child have the fd /opt/my/sensitive/file open
jandre@ubuntu:/tmp$ lsof | grep sensitive
node 39056 jandre 9w REG 8,1 0 827178 /opt/my/sensitive/file
native_binary 39058 jandre 9w REG 8,1 0 827178 /opt/my/sensitive/file
For reference, the node.js code I wrote did something like this. Doot doot, super-benign-looking:
var spawn = require(‘child_process’).spawn;
var fs = require(‘fs’);
// some other things happen (...)
var fd = fs.openSync(‘/opt/my/sensitive/file’, ‘r’);
// some more things happen! (...)
spawn(‘native_binary’, [ args ], opts); // hi, i'm spawning a child process
I concluded right away that the output of `lsof` was Very Bad. Let’s look at the fork(2) page for some answers:
The child inherits copies of the parent’s set of open file descriptors. Each file descriptor in the child refers to the same open file description (see open(2)) as the corresponding file descriptor in the parent.
This means that by default (unless you set some special flags as I talk about below), any process you launch from a parent process will inherit all of its open files. There are reasons it does this, as it is very convenient for inter-process communication, but it is also has the potential to be a big problem. Why?
- Your processes only get allocated so many open file descriptors. `ulimit -n` will show you what your restrictions are. If you are opening n files and spawning m child processes, which are cloning the file descriptors, you n * m more file descriptors than you should be. You may encounter a ‘EMFILE, too many open files’ issue sooner than you’d expect.
- It can be a security problem. Think about if you were writing a small ‘sshd’-like clone. Your parent process, which runs as root so it can do root-like things, has an open file descriptor reading from ‘/etc/shadow’ to authenticate the users. When a user logs in, it spawns ‘/bin/sh’ shells for each user after they’ve authenticated. Now, if you smart, you spawn the shell process and setreuid to ensure the user’s shell can’t perform actions as root, only its own user account, but alas — unless you specify otherwise, those shells will have open file descriptors to access to ‘/etc/shadow’ with the same permissions as its root-running parent process!
Now, there are two ways to avoid having the file descriptors being inherited, and I wanted to make sure that I wasn’t just missing some configuration option or API parameter to set those in Node’s API. Python, for example, has a ‘close_fds’ parameter you can toggle when creating new processes.
Option #1) If you are writing your code in C, after opening your file, you can use the fcntl(2) function to set FD_CLOEXEC on the file descriptor, effectively telling your system, ‘don’t keep this file open when you execve() a new process’).
- I grepped through the node source and found a wrapper for this in libuv, though it’s not exposed: https://github.com/joyent/libuv/blob/master/src/unix/core.c#L499-L528.
Option #2) Again, in C, when you call open(3) on the file, you can set O_CLOEXEC to have that ‘don’t keep this file open on execve’ flag set automatically right when you open it.
- I wanted to make sure this wasn’t somehow exposed already in node’s fs.open wrapper flags, but found no evidence that it was.
Option #3) Use fopen(file, “e”) (since glibc 2.7) ** EDIT ADDED BY MARK WHO IS VERY PEDANTIC ABOUT THESE THINGS
- Close files where you can if you are spawning processes in Node.js. If not, you can manually pass numeric flags to fs.open, rather than the ‘w’, ‘r+’, etc, abstraction, so you should do so if you need O_CLOEXEC. Or, you can write a small c++ addon that wraps fcntl().
- In particular, you should be very careful about this if you have sensitive code and are designing it to ‘drop privileges’. This isn’t a theoretical vulnerability, bugs arisen from FD leaks have actually been exploited before.
- Engineering is hard, and so is security. Lots of people have been using Node.js without this issue coming to light, not to mention that the guys maintaining node and libuv are very smart. Despite my sensationalist title, this is the kind of issue any good engineer could overlook, and becomes harder and harder to unearth the more complex a system becomes.
Interested in security, systems, and writing software? Live in Boston, or want to move here? We are hiring at Threat Stack!