diff --git a/src/unix/pty.cc b/src/unix/pty.cc index 3b7d3dc1c..01fb67fce 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -112,8 +112,6 @@ struct ExitEvent { int exit_code = 0, signal_code = 0; }; -#if defined(__linux__) - static int SetCloseOnExec(int fd) { int flags = fcntl(fd, F_GETFD, 0); @@ -124,6 +122,8 @@ SetCloseOnExec(int fd) { return fcntl(fd, F_SETFD, flags | FD_CLOEXEC); } +#if defined(__linux__) + /** * Close all file descriptors >= 3 to prevent FD leakage to child processes. * Uses close_range() syscall on Linux 5.9+, falls back to /proc/self/fd iteration. @@ -415,6 +415,9 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); } + if (SetCloseOnExec(master) == -1) { + throw Napi::Error::New(napiEnv, "Could not set master fd to close-on-exec."); + } #else int argc = argv_.Length(); int argl = argc + 2; @@ -488,6 +491,9 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) { if (pty_nonblock(master) == -1) { throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking."); } + if (SetCloseOnExec(master) == -1) { + throw Napi::Error::New(napiEnv, "Could not set master fd to close-on-exec."); + } } #endif @@ -535,6 +541,10 @@ Napi::Value PtyOpen(const Napi::CallbackInfo& info) { throw Napi::Error::New(env, "Could not set slave fd to nonblocking."); } + if (SetCloseOnExec(master) == -1 || SetCloseOnExec(slave) == -1) { + throw Napi::Error::New(env, "Could not set pty fds to close-on-exec."); + } + Napi::Object obj = Napi::Object::New(env); obj.Set("master", Napi::Number::New(env, master)); obj.Set("slave", Napi::Number::New(env, slave)); diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index a666e91aa..4abbd243d 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -112,6 +112,14 @@ if (process.platform !== 'win32') { term.slave!.write('slave\n'); term.master!.write('master\n'); }); + if (process.platform === 'linux') { + it('should not leak the master or slave fd to processes the host spawns', () => { + term = UnixTerminal.open({}); + const fds = cp.execFileSync('/bin/sh', ['-c', 'ls -l /proc/$$/fd'], { stdio: ['ignore', 'pipe', 'ignore'] }).toString(); + assert.ok(!fds.includes('ptmx'), `child holds a pty master:\n${fds}`); + assert.ok(!fds.includes(term.ptsName), `child holds the pty slave ${term.ptsName}:\n${fds}`); + }); + } }); describe('close', () => { const term = new UnixTerminal('node'); @@ -287,6 +295,20 @@ if (process.platform !== 'win32') { done(); }, 1000); }); + it('should not leak pty master fds to processes the host spawns', () => { + const ptys: UnixTerminalType[] = []; + for (let i = 0; i < 2; i++) { + ptys.push(new UnixTerminal('/bin/bash', [], {})); + } + try { + const fds = cp.execFileSync('/bin/sh', ['-c', 'ls -l /proc/$$/fd'], { stdio: ['ignore', 'pipe', 'ignore'] }).toString(); + assert.ok(!fds.includes('ptmx'), `child holds a pty master:\n${fds}`); + } finally { + for (const pty of ptys) { + pty.kill(); + } + } + }); } if (process.platform === 'darwin') { it('should return the name of the process', (done) => {