From e326c4a9d429c65c2263abc9815e7cdbeba38345 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Mon, 9 Aug 2021 21:46:45 +0200 Subject: [PATCH 01/17] posix_spawn: added helper --- binding.gyp | 70 +++++++---- scripts/post-install.js | 1 + src/native.d.ts | 2 +- src/unix/comms.cc | 55 ++++++++ src/unix/comms.h | 19 +++ src/unix/pty.cc | 264 ++++++++++++++++----------------------- src/unix/spawn-helper.cc | 82 ++++++++++++ src/unixTerminal.test.ts | 10 ++ src/unixTerminal.ts | 8 +- 9 files changed, 325 insertions(+), 186 deletions(-) create mode 100644 src/unix/comms.cc create mode 100644 src/unix/comms.h create mode 100644 src/unix/spawn-helper.cc diff --git a/binding.gyp b/binding.gyp index 46bb70d26..9d548d9c7 100644 --- a/binding.gyp +++ b/binding.gyp @@ -46,33 +46,49 @@ } ] }, { # OS!="win" - 'targets': [{ - 'target_name': 'pty', - 'include_dirs' : [ - ' void): IUnixProcess; + fork(file: string, args: string[], parsedEnv: string[], cwd: string, cols: number, rows: number, uid: number, gid: number, useUtf8: boolean, onExitCallback: (code: number, signal: number) => void, helperPath: string): IUnixProcess; open(cols: number, rows: number): IUnixOpenProcess; process(fd: number, pty: string): string; resize(fd: number, cols: number, rows: number): void; diff --git a/src/unix/comms.cc b/src/unix/comms.cc new file mode 100644 index 000000000..a1e4c530d --- /dev/null +++ b/src/unix/comms.cc @@ -0,0 +1,55 @@ +#include "comms.h" + +#include +#include +#include + + +void comm_send_int(int fd, int data) { + send(fd, &data, sizeof(data), 0); +} + +void comm_send_str(int fd, char *str) { + comm_send_int(fd, strlen(str)); + send(fd, str, strlen(str), 0); +} + +void comm_send_str_array(int fd, char **arr) { + int len = 0; + auto p = arr; + while (*p) { + len++; + p++; + } + + comm_send_int(fd, len); + p = arr; + while (*p) { + comm_send_str(fd, *p); + p++; + } +} + +int comm_recv_int(int fd) { + int buf; + recv(fd, &buf, sizeof(buf), 0); + return buf; +} + +char* comm_recv_str(int fd) { + int len = comm_recv_int(fd); + auto buf = new char[len + 1]; + recv(fd, buf, len, 0); + buf[len] = 0; + return buf; +} + +char** comm_recv_str_array(int fd) { + int len = comm_recv_int(fd); + auto buf = new char*[len + 1]; + buf[len] = nullptr; + for (int i = 0; i < len; i++) { + buf[i] = comm_recv_str(fd); + } + return buf; +} diff --git a/src/unix/comms.h b/src/unix/comms.h new file mode 100644 index 000000000..49cb78fef --- /dev/null +++ b/src/unix/comms.h @@ -0,0 +1,19 @@ +#define COMM_PTY_FD 0 +#define COMM_PIPE_FD 1 + +#define COMM_MSG_PATH 1 +#define COMM_MSG_ARGV 2 +#define COMM_MSG_ENV 3 +#define COMM_MSG_CWD 4 +#define COMM_MSG_UID 5 +#define COMM_MSG_GID 6 +#define COMM_MSG_GO_FOR_LAUNCH 99 + +#define COMM_MSG_EXEC_ERROR 100 + +void comm_send_int(int fd, int data); +void comm_send_str(int fd, char *str); +void comm_send_str_array(int fd, char **arr); +int comm_recv_int(int fd); +char* comm_recv_str(int fd); +char** comm_recv_str_array(int fd); diff --git a/src/unix/pty.cc b/src/unix/pty.cc index ce44a3e69..c5837e4e0 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -29,6 +29,9 @@ #include #include #include +#include + +#include "comms.h" /* forkpty */ /* http://www.gnu.org/software/gnulib/manual/html_node/forkpty.html */ @@ -54,15 +57,6 @@ #define VDISCARD VDISCRD #endif -/* environ for execvpe */ -/* node/src/node_child_process.cc */ -#if defined(__APPLE__) && !TARGET_OS_IPHONE -#include -#define environ (*_NSGetEnviron()) -#else -extern char **environ; -#endif - /* for pty_getproc */ #if defined(__linux__) #include @@ -77,6 +71,10 @@ extern char **environ; #define NSIG 32 #endif +#ifndef POSIX_SPAWN_CLOEXEC_DEFAULT + #define POSIX_SPAWN_CLOEXEC_DEFAULT 0 +#endif + /** * Structs */ @@ -103,9 +101,6 @@ NAN_METHOD(PtyGetProc); * Functions */ -static int -pty_execvpe(const char *, char **, char **); - static int pty_nonblock(int); @@ -117,11 +112,6 @@ pty_openpty(int *, int *, char *, const struct termios *, const struct winsize *); -static pid_t -pty_forkpty(int *, char *, - const struct termios *, - const struct winsize *); - static void pty_waitpid(void *); @@ -134,7 +124,7 @@ pty_after_close(uv_handle_t *); NAN_METHOD(PtyFork) { Nan::HandleScope scope; - if (info.Length() != 10 || + if (info.Length() != 11 || !info[0]->IsString() || !info[1]->IsArray() || !info[2]->IsArray() || @@ -144,9 +134,10 @@ NAN_METHOD(PtyFork) { !info[6]->IsNumber() || !info[7]->IsNumber() || !info[8]->IsBoolean() || - !info[9]->IsFunction()) { + !info[9]->IsFunction() || + !info[10]->IsString()) { return Nan::ThrowError( - "Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, utf8, onexit)"); + "Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, utf8, onexit, helperPath)"); } // file @@ -229,11 +220,11 @@ NAN_METHOD(PtyFork) { int uid = info[6]->IntegerValue(Nan::GetCurrentContext()).FromJust(); int gid = info[7]->IntegerValue(Nan::GetCurrentContext()).FromJust(); - // fork the pty - int master = -1; + // helperPath + Nan::Utf8String helper_path_(info[10]); + char *helper_path = strdup(*helper_path_); sigset_t newmask, oldmask; - struct sigaction sig_action; // temporarily block all signals // this is needed due to a race condition in openpty @@ -242,85 +233,108 @@ NAN_METHOD(PtyFork) { sigfillset(&newmask); pthread_sigmask(SIG_SETMASK, &newmask, &oldmask); - pid_t pid = pty_forkpty(&master, nullptr, term, &winp); + int master, slave; + int ret = pty_openpty(&master, &slave, nullptr, NULL, &winp); + if (ret == -1) { + perror("openpty failed"); + return Nan::ThrowError("openpty failed."); + } - if (!pid) { - // remove all signal handler from child - sig_action.sa_handler = SIG_DFL; - sig_action.sa_flags = 0; - sigemptyset(&sig_action.sa_mask); - for (int i = 0 ; i < NSIG ; i++) { // NSIG is a macro for all signals + 1 - sigaction(i, &sig_action, NULL); - } + int comms_pipe[2]; + if (socketpair(AF_UNIX, SOCK_STREAM, 0, comms_pipe)) { + perror("socketpair failed"); + return Nan::ThrowError("socketpair failed."); } + + posix_spawn_file_actions_t acts; + posix_spawn_file_actions_init(&acts); + posix_spawn_file_actions_adddup2(&acts, slave, COMM_PTY_FD); + posix_spawn_file_actions_addclose(&acts, master); + posix_spawn_file_actions_addclose(&acts, slave); + posix_spawn_file_actions_adddup2(&acts, comms_pipe[1], COMM_PIPE_FD); + posix_spawn_file_actions_addclose(&acts, comms_pipe[0]); + + posix_spawnattr_t attrs; + posix_spawnattr_init(&attrs); + posix_spawnattr_setflags(&attrs, POSIX_SPAWN_CLOEXEC_DEFAULT); + + pid_t pid; + auto error = posix_spawn(&pid, helper_path, &acts, &attrs, argv, env); + + posix_spawn_file_actions_destroy(&acts); + posix_spawnattr_destroy(&attrs); + close(comms_pipe[1]); + // reenable signals pthread_sigmask(SIG_SETMASK, &oldmask, NULL); - if (pid) { - for (i = 0; i < argl; i++) free(argv[i]); - delete[] argv; - for (i = 0; i < envc; i++) free(env[i]); - delete[] env; - free(cwd); + if (error) { + perror("posix_spawn failed"); + return Nan::ThrowError("posix_spawn(3) failed."); } - switch (pid) { - case -1: - return Nan::ThrowError("forkpty(3) failed."); - case 0: - if (strlen(cwd)) { - if (chdir(cwd) == -1) { - perror("chdir(2) failed."); - _exit(1); - } - } - - if (uid != -1 && gid != -1) { - if (setgid(gid) == -1) { - perror("setgid(2) failed."); - _exit(1); - } - if (setuid(uid) == -1) { - perror("setuid(2) failed."); - _exit(1); - } - } - - pty_execvpe(argv[0], argv, env); - - perror("execvp(3) failed."); - _exit(1); - default: - if (pty_nonblock(master) == -1) { - return Nan::ThrowError("Could not set master fd to nonblocking."); - } - - v8::Local obj = Nan::New(); - Nan::Set(obj, - Nan::New("fd").ToLocalChecked(), - Nan::New(master)); - Nan::Set(obj, - Nan::New("pid").ToLocalChecked(), - Nan::New(pid)); - Nan::Set(obj, - Nan::New("pty").ToLocalChecked(), - Nan::New(ptsname(master)).ToLocalChecked()); - - pty_baton *baton = new pty_baton(); - baton->exit_code = 0; - baton->signal_code = 0; - baton->cb.Reset(v8::Local::Cast(info[9])); - baton->pid = pid; - baton->async.data = baton; - - uv_async_init(uv_default_loop(), &baton->async, pty_after_waitpid); - - uv_thread_create(&baton->tid, pty_waitpid, static_cast(baton)); - - return info.GetReturnValue().Set(obj); + comm_send_int(comms_pipe[0], COMM_MSG_PATH); + comm_send_str(comms_pipe[0], argv[0]); + comm_send_int(comms_pipe[0], COMM_MSG_ARGV); + comm_send_str_array(comms_pipe[0], argv); + comm_send_int(comms_pipe[0], COMM_MSG_ENV); + comm_send_str_array(comms_pipe[0], env); + if (strlen(cwd)) { + comm_send_int(comms_pipe[0], COMM_MSG_CWD); + comm_send_str(comms_pipe[0], cwd); + } + if (uid != -1) { + comm_send_int(comms_pipe[0], COMM_MSG_UID); + comm_send_int(comms_pipe[0], uid); } + if (gid != -1) { + comm_send_int(comms_pipe[0], COMM_MSG_GID); + comm_send_int(comms_pipe[0], gid); + } + comm_send_int(comms_pipe[0], COMM_MSG_GO_FOR_LAUNCH); - return info.GetReturnValue().SetUndefined(); + int result; + auto bytes_read = recv(comms_pipe[0], &result, sizeof(result), 0); + close(comms_pipe[0]); + + for (i = 0; i < argl; i++) free(argv[i]); + delete[] argv; + for (i = 0; i < envc; i++) free(env[i]); + delete[] env; + free(cwd); + free(helper_path); + + if (bytes_read && result == COMM_MSG_EXEC_ERROR) { + return Nan::ThrowError("exec error"); + } + + if (pty_nonblock(master) == -1) { + return Nan::ThrowError("Could not set master fd to nonblocking."); + } + + v8::Local obj = Nan::New(); + Nan::Set(obj, + Nan::New("fd").ToLocalChecked(), + Nan::New(master)); + Nan::Set(obj, + Nan::New("pid").ToLocalChecked(), + Nan::New(pid)); + Nan::Set(obj, + Nan::New("pty").ToLocalChecked(), + Nan::New(ptsname(master)).ToLocalChecked()); + + pty_baton *baton = new pty_baton(); + baton->exit_code = 0; + baton->signal_code = 0; + baton->cb.Reset(v8::Local::Cast(info[9])); + baton->pid = pid; + baton->async.data = baton; + + uv_async_init(uv_default_loop(), &baton->async, pty_after_waitpid); + + uv_thread_create(&baton->tid, pty_waitpid, static_cast(baton)); + + return info.GetReturnValue().Set(obj); } NAN_METHOD(PtyOpen) { @@ -428,21 +442,6 @@ NAN_METHOD(PtyGetProc) { return info.GetReturnValue().Set(name_); } -/** - * execvpe - */ - -// execvpe(3) is not portable. -// http://www.gnu.org/software/gnulib/manual/html_node/execvpe.html -static int -pty_execvpe(const char *file, char **argv, char **envp) { - char **old = environ; - environ = envp; - int ret = execvp(file, argv); - environ = old; - return ret; -} - /** * Nonblocking FD */ @@ -670,55 +669,6 @@ pty_openpty(int *amaster, #endif } -static pid_t -pty_forkpty(int *amaster, - char *name, - const struct termios *termp, - const struct winsize *winp) { -#if defined(__sun) - int master, slave; - - int ret = pty_openpty(&master, &slave, name, termp, winp); - if (ret == -1) return -1; - if (amaster) *amaster = master; - - pid_t pid = fork(); - - switch (pid) { - case -1: // error in fork, we are still in parent - close(master); - close(slave); - return -1; - case 0: // we are in the child process - close(master); - setsid(); - -#if defined(TIOCSCTTY) - // glibc does this - if (ioctl(slave, TIOCSCTTY, NULL) == -1) { - _exit(1); - } -#endif - - dup2(slave, 0); - dup2(slave, 1); - dup2(slave, 2); - - if (slave > 2) close(slave); - - return 0; - default: // we are in the parent process - close(slave); - return pid; - } - - return -1; -#else - return forkpty(amaster, name, (termios *)termp, (winsize *)winp); -#endif -} - - /** * Init */ diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc new file mode 100644 index 000000000..1eb69d86b --- /dev/null +++ b/src/unix/spawn-helper.cc @@ -0,0 +1,82 @@ +#include +#include +#include +#include +#include +#include +#include + +#include "comms.h" + +#define PIPE_FD (STDERR_FILENO + 1) + +/* environ for execvpe */ +/* node/src/node_child_process.cc */ +#if defined(__APPLE__) && !TARGET_OS_IPHONE + #include + #define environ (*_NSGetEnviron()) +#else + extern char **environ; +#endif + +int main () { + sigset_t empty_set; + sigemptyset(&empty_set); + pthread_sigmask(SIG_SETMASK, &empty_set, nullptr); + + setsid(); + + dup2(COMM_PIPE_FD, PIPE_FD); + close(COMM_PIPE_FD); + dup2(COMM_PTY_FD, STDIN_FILENO); + dup2(COMM_PTY_FD, STDOUT_FILENO); + dup2(COMM_PTY_FD, STDERR_FILENO); + +#if defined(TIOCSCTTY) + // glibc does this + if (ioctl(STDIN_FILENO, TIOCSCTTY, NULL) == -1) { + _exit(1); + } +#endif + + char *file = nullptr; + char **argv = nullptr; + char **env = nullptr; + + while (true) { + const int type = comm_recv_int(PIPE_FD); + if (type == COMM_MSG_PATH) { + file = comm_recv_str(PIPE_FD); + } else if (type == COMM_MSG_ARGV) { + argv = comm_recv_str_array(PIPE_FD); + } else if (type == COMM_MSG_ENV) { + env = comm_recv_str_array(PIPE_FD); + } else if (type == COMM_MSG_CWD) { + if (chdir(comm_recv_str(PIPE_FD)) == -1) { + perror("chdir(2) failed."); + _exit(1); + } + } else if (type == COMM_MSG_UID) { + if (setuid(comm_recv_int(PIPE_FD)) == -1) { + perror("setuid(2) failed."); + _exit(1); + } + } else if (type == COMM_MSG_GID) { + if (setgid(comm_recv_int(PIPE_FD)) == -1) { + perror("setgid(2) failed."); + _exit(1); + } + } else if (type == COMM_MSG_GO_FOR_LAUNCH) { + break; + } + } + + environ = env; + fcntl(PIPE_FD, F_SETFD, FD_CLOEXEC); + + auto error = execvp(file, argv); + + comm_send_int(PIPE_FD, COMM_MSG_EXEC_ERROR); + comm_send_int(PIPE_FD, error); + return 1; +} diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index ee2cd6964..9fec50eeb 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -239,5 +239,15 @@ if (process.platform !== 'win32') { }); }); }); + describe('spawn', () => { + it('should handle exec() errors', (done) => { + try { + new UnixTerminal('/bin/bogus.exe', []); + done(new Error('should have failed')); + } catch { + done(); + } + }); + }); }); } diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 690dbb576..576270af9 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -4,17 +4,21 @@ * Copyright (c) 2018, Microsoft Corporation (MIT License). */ import * as net from 'net'; +import * as path from 'path'; import { Terminal, DEFAULT_COLS, DEFAULT_ROWS } from './terminal'; import { IProcessEnv, IPtyForkOptions, IPtyOpenOptions } from './interfaces'; import { ArgvOrCommandLine } from './types'; import { assign } from './utils'; let pty: IUnixNative; +let helperPath: string; try { pty = require('../build/Release/pty.node'); + helperPath = '../build/Release/spawn-helper'; } catch (outerError) { try { pty = require('../build/Debug/pty.node'); + helperPath = '../build/Debug/spawn-helper'; } catch (innerError) { console.error('innerError', innerError); // Re-throw the exception from the Release require if the Debug require fails as well @@ -22,6 +26,8 @@ try { } } +helperPath = path.resolve(__dirname, helperPath); + const DEFAULT_FILE = 'sh'; const DEFAULT_NAME = 'xterm'; const DESTROY_SOCKET_TIMEOUT_MS = 200; @@ -103,7 +109,7 @@ export class UnixTerminal extends Terminal { }; // fork - const term = pty.fork(file, args, parsedEnv, cwd, this._cols, this._rows, uid, gid, (encoding === 'utf8'), onexit); + const term = pty.fork(file, args, parsedEnv, cwd, this._cols, this._rows, uid, gid, (encoding === 'utf8'), onexit, helperPath); this._socket = new PipeSocket(term.fd); if (encoding !== null) { From 63a34195c17b6bed91eb6899934ddfb9b03a18b9 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Mon, 9 Aug 2021 22:03:53 +0200 Subject: [PATCH 02/17] close all open files --- src/unix/spawn-helper.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index 1eb69d86b..eee08291b 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include @@ -24,6 +25,14 @@ int main () { sigemptyset(&empty_set); pthread_sigmask(SIG_SETMASK, &empty_set, nullptr); + struct rlimit rlim_ofile; + getrlimit(RLIMIT_NOFILE, &rlim_ofile); + for (rlim_t fd = 0; fd < rlim_ofile.rlim_cur; fd++) { + if (fd != COMM_PIPE_FD && fd != COMM_PTY_FD) { + close(fd); + } + } + setsid(); dup2(COMM_PIPE_FD, PIPE_FD); From def8f6175f9cc799442e6bec76fbd2c14050a44c Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Mon, 9 Aug 2021 23:04:57 +0200 Subject: [PATCH 03/17] posix_spawn: re-added missing term --- src/unix/pty.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index c5837e4e0..23e12e051 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -234,7 +234,7 @@ NAN_METHOD(PtyFork) { pthread_sigmask(SIG_SETMASK, &newmask, &oldmask); int master, slave; - int ret = pty_openpty(&master, &slave, nullptr, NULL, &winp); + int ret = pty_openpty(&master, &slave, nullptr, term, &winp); if (ret == -1) { perror("openpty failed"); return Nan::ThrowError("openpty failed."); @@ -256,7 +256,7 @@ NAN_METHOD(PtyFork) { posix_spawnattr_t attrs; posix_spawnattr_init(&attrs); - posix_spawnattr_setflags(&attrs, POSIX_SPAWN_CLOEXEC_DEFAULT); + posix_spawnattr_setflags(&attrs, POSIX_SPAWN_RESETIDS | POSIX_SPAWN_CLOEXEC_DEFAULT); pid_t pid; auto error = posix_spawn(&pid, helper_path, &acts, &attrs, argv, env); @@ -292,6 +292,7 @@ NAN_METHOD(PtyFork) { comm_send_int(comms_pipe[0], gid); } comm_send_int(comms_pipe[0], COMM_MSG_GO_FOR_LAUNCH); + shutdown(comms_pipe[0], SHUT_WR); int result; auto bytes_read = recv(comms_pipe[0], &result, sizeof(result), 0); From 9abc91358df02014f74880ce9a78cafeda4eb117 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Tue, 10 Aug 2021 15:17:16 +0200 Subject: [PATCH 04/17] dropped socket communication --- src/unix/comms.cc | 55 ------------------------- src/unix/comms.h | 20 +-------- src/unix/pty.cc | 87 +++++++++++++++------------------------- src/unix/spawn-helper.cc | 77 +++++++++++++---------------------- 4 files changed, 61 insertions(+), 178 deletions(-) delete mode 100644 src/unix/comms.cc diff --git a/src/unix/comms.cc b/src/unix/comms.cc deleted file mode 100644 index a1e4c530d..000000000 --- a/src/unix/comms.cc +++ /dev/null @@ -1,55 +0,0 @@ -#include "comms.h" - -#include -#include -#include - - -void comm_send_int(int fd, int data) { - send(fd, &data, sizeof(data), 0); -} - -void comm_send_str(int fd, char *str) { - comm_send_int(fd, strlen(str)); - send(fd, str, strlen(str), 0); -} - -void comm_send_str_array(int fd, char **arr) { - int len = 0; - auto p = arr; - while (*p) { - len++; - p++; - } - - comm_send_int(fd, len); - p = arr; - while (*p) { - comm_send_str(fd, *p); - p++; - } -} - -int comm_recv_int(int fd) { - int buf; - recv(fd, &buf, sizeof(buf), 0); - return buf; -} - -char* comm_recv_str(int fd) { - int len = comm_recv_int(fd); - auto buf = new char[len + 1]; - recv(fd, buf, len, 0); - buf[len] = 0; - return buf; -} - -char** comm_recv_str_array(int fd) { - int len = comm_recv_int(fd); - auto buf = new char*[len + 1]; - buf[len] = nullptr; - for (int i = 0; i < len; i++) { - buf[i] = comm_recv_str(fd); - } - return buf; -} diff --git a/src/unix/comms.h b/src/unix/comms.h index 49cb78fef..8d0cf543f 100644 --- a/src/unix/comms.h +++ b/src/unix/comms.h @@ -1,19 +1 @@ -#define COMM_PTY_FD 0 -#define COMM_PIPE_FD 1 - -#define COMM_MSG_PATH 1 -#define COMM_MSG_ARGV 2 -#define COMM_MSG_ENV 3 -#define COMM_MSG_CWD 4 -#define COMM_MSG_UID 5 -#define COMM_MSG_GID 6 -#define COMM_MSG_GO_FOR_LAUNCH 99 - -#define COMM_MSG_EXEC_ERROR 100 - -void comm_send_int(int fd, int data); -void comm_send_str(int fd, char *str); -void comm_send_str_array(int fd, char **arr); -int comm_recv_int(int fd); -char* comm_recv_str(int fd); -char** comm_recv_str_array(int fd); +#define COMM_PIPE_FD (STDERR_FILENO + 1) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index 23e12e051..7c00b7f99 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -143,33 +143,39 @@ NAN_METHOD(PtyFork) { // file Nan::Utf8String file(info[0]); - // args - int i = 0; - v8::Local argv_ = v8::Local::Cast(info[1]); - int argc = argv_->Length(); - int argl = argc + 1 + 1; - char **argv = new char*[argl]; - argv[0] = strdup(*file); - argv[argl-1] = NULL; - for (; i < argc; i++) { - Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked()); - argv[i+1] = strdup(*arg); - } - // env - i = 0; v8::Local env_ = v8::Local::Cast(info[2]); int envc = env_->Length(); char **env = new char*[envc+1]; env[envc] = NULL; - for (; i < envc; i++) { + for (int i = 0; i < envc; i++) { Nan::Utf8String pair(Nan::Get(env_, i).ToLocalChecked()); env[i] = strdup(*pair); } // cwd Nan::Utf8String cwd_(info[3]); - char *cwd = strdup(*cwd_); + + // uid / gid + int uid = info[6]->IntegerValue(Nan::GetCurrentContext()).FromJust(); + int gid = info[7]->IntegerValue(Nan::GetCurrentContext()).FromJust(); + + // args + v8::Local argv_ = v8::Local::Cast(info[1]); + + const int EXTRA_ARGS = 4; + int argc = argv_->Length(); + int argl = argc + EXTRA_ARGS + 1; + char **argv = new char*[argl]; + argv[0] = strdup(*cwd_); + argv[1] = strdup(std::to_string(uid).c_str()); + argv[2] = strdup(std::to_string(gid).c_str()); + argv[3] = strdup(*file); + argv[argl - 1] = NULL; + for (int i = 0; i < argc; i++) { + Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked()); + argv[i + EXTRA_ARGS] = strdup(*arg); + } // size struct winsize winp; @@ -216,10 +222,6 @@ NAN_METHOD(PtyFork) { cfsetispeed(term, B38400); cfsetospeed(term, B38400); - // uid / gid - int uid = info[6]->IntegerValue(Nan::GetCurrentContext()).FromJust(); - int gid = info[7]->IntegerValue(Nan::GetCurrentContext()).FromJust(); - // helperPath Nan::Utf8String helper_path_(info[10]); char *helper_path = strdup(*helper_path_); @@ -241,18 +243,18 @@ NAN_METHOD(PtyFork) { } int comms_pipe[2]; - if (socketpair(AF_UNIX, SOCK_STREAM, 0, comms_pipe)) { - perror("socketpair failed"); - return Nan::ThrowError("socketpair failed."); + if (pipe(comms_pipe)) { + perror("pipe() failed"); + return Nan::ThrowError("pipe() failed."); } posix_spawn_file_actions_t acts; posix_spawn_file_actions_init(&acts); - posix_spawn_file_actions_adddup2(&acts, slave, COMM_PTY_FD); - posix_spawn_file_actions_addclose(&acts, master); - posix_spawn_file_actions_addclose(&acts, slave); + posix_spawn_file_actions_adddup2(&acts, slave, STDIN_FILENO); + posix_spawn_file_actions_adddup2(&acts, slave, STDOUT_FILENO); + posix_spawn_file_actions_adddup2(&acts, slave, STDERR_FILENO); posix_spawn_file_actions_adddup2(&acts, comms_pipe[1], COMM_PIPE_FD); - posix_spawn_file_actions_addclose(&acts, comms_pipe[0]); + posix_spawn_file_actions_addclose(&acts, comms_pipe[1]); posix_spawnattr_t attrs; posix_spawnattr_init(&attrs); @@ -273,39 +275,16 @@ NAN_METHOD(PtyFork) { return Nan::ThrowError("posix_spawn(3) failed."); } - comm_send_int(comms_pipe[0], COMM_MSG_PATH); - comm_send_str(comms_pipe[0], argv[0]); - comm_send_int(comms_pipe[0], COMM_MSG_ARGV); - comm_send_str_array(comms_pipe[0], argv); - comm_send_int(comms_pipe[0], COMM_MSG_ENV); - comm_send_str_array(comms_pipe[0], env); - if (strlen(cwd)) { - comm_send_int(comms_pipe[0], COMM_MSG_CWD); - comm_send_str(comms_pipe[0], cwd); - } - if (uid != -1) { - comm_send_int(comms_pipe[0], COMM_MSG_UID); - comm_send_int(comms_pipe[0], uid); - } - if (gid != -1) { - comm_send_int(comms_pipe[0], COMM_MSG_GID); - comm_send_int(comms_pipe[0], gid); - } - comm_send_int(comms_pipe[0], COMM_MSG_GO_FOR_LAUNCH); - shutdown(comms_pipe[0], SHUT_WR); - - int result; - auto bytes_read = recv(comms_pipe[0], &result, sizeof(result), 0); + auto bytes_read = read(comms_pipe[0], &error, sizeof(error)); close(comms_pipe[0]); - for (i = 0; i < argl; i++) free(argv[i]); + for (int i = 0; i < argl; i++) free(argv[i]); delete[] argv; - for (i = 0; i < envc; i++) free(env[i]); + for (int i = 0; i < envc; i++) free(env[i]); delete[] env; - free(cwd); free(helper_path); - if (bytes_read && result == COMM_MSG_EXEC_ERROR) { + if (bytes_read && error) { return Nan::ThrowError("exec error"); } diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index eee08291b..169273a6e 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -1,16 +1,15 @@ #include +#include +#include +#include #include -#include #include -#include +#include #include #include -#include #include "comms.h" -#define PIPE_FD (STDERR_FILENO + 1) - /* environ for execvpe */ /* node/src/node_child_process.cc */ #if defined(__APPLE__) && !TARGET_OS_IPHONE @@ -20,27 +19,21 @@ extern char **environ; #endif -int main () { +int main (int argc, char** argv) { sigset_t empty_set; sigemptyset(&empty_set); pthread_sigmask(SIG_SETMASK, &empty_set, nullptr); struct rlimit rlim_ofile; getrlimit(RLIMIT_NOFILE, &rlim_ofile); - for (rlim_t fd = 0; fd < rlim_ofile.rlim_cur; fd++) { - if (fd != COMM_PIPE_FD && fd != COMM_PTY_FD) { + for (rlim_t fd = STDERR_FILENO + 1; fd < rlim_ofile.rlim_cur; fd++) { + if (fd != COMM_PIPE_FD) { close(fd); } } setsid(); - dup2(COMM_PIPE_FD, PIPE_FD); - close(COMM_PIPE_FD); - dup2(COMM_PTY_FD, STDIN_FILENO); - dup2(COMM_PTY_FD, STDOUT_FILENO); - dup2(COMM_PTY_FD, STDERR_FILENO); - #if defined(TIOCSCTTY) // glibc does this if (ioctl(STDIN_FILENO, TIOCSCTTY, NULL) == -1) { @@ -48,44 +41,28 @@ int main () { } #endif - char *file = nullptr; - char **argv = nullptr; - char **env = nullptr; + char *cwd = argv[0]; + int uid = std::stoi(argv[1]); + int gid = std::stoi(argv[2]); + char *file = argv[3]; + argv = &argv[3]; - while (true) { - const int type = comm_recv_int(PIPE_FD); - if (type == COMM_MSG_PATH) { - file = comm_recv_str(PIPE_FD); - } else if (type == COMM_MSG_ARGV) { - argv = comm_recv_str_array(PIPE_FD); - } else if (type == COMM_MSG_ENV) { - env = comm_recv_str_array(PIPE_FD); - } else if (type == COMM_MSG_CWD) { - if (chdir(comm_recv_str(PIPE_FD)) == -1) { - perror("chdir(2) failed."); - _exit(1); - } - } else if (type == COMM_MSG_UID) { - if (setuid(comm_recv_int(PIPE_FD)) == -1) { - perror("setuid(2) failed."); - _exit(1); - } - } else if (type == COMM_MSG_GID) { - if (setgid(comm_recv_int(PIPE_FD)) == -1) { - perror("setgid(2) failed."); - _exit(1); - } - } else if (type == COMM_MSG_GO_FOR_LAUNCH) { - break; - } - } - - environ = env; - fcntl(PIPE_FD, F_SETFD, FD_CLOEXEC); + fcntl(COMM_PIPE_FD, F_SETFD, FD_CLOEXEC); - auto error = execvp(file, argv); + if (strlen(cwd) && chdir(cwd) == -1) { + perror("chdir(2) failed."); + _exit(1); + } + if (uid != -1 && setuid(uid) == -1) { + perror("setuid(2) failed."); + _exit(1); + } + if (gid != -1 && setgid(gid) == -1) { + perror("setgid(2) failed."); + _exit(1); + } - comm_send_int(PIPE_FD, COMM_MSG_EXEC_ERROR); - comm_send_int(PIPE_FD, error); + execvp(file, argv); + write(COMM_PIPE_FD, &errno, sizeof(errno)); return 1; } From 29b5a56b6832ea45ad5f345f68f87f0344ef61ee Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Tue, 10 Aug 2021 15:21:47 +0200 Subject: [PATCH 05/17] posix_spawn: plugged memory leaks --- src/unix/pty.cc | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index 7c00b7f99..9052b9490 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -239,13 +239,15 @@ NAN_METHOD(PtyFork) { int ret = pty_openpty(&master, &slave, nullptr, term, &winp); if (ret == -1) { perror("openpty failed"); - return Nan::ThrowError("openpty failed."); + Nan::ThrowError("openpty failed."); + goto done; } int comms_pipe[2]; if (pipe(comms_pipe)) { perror("pipe() failed"); - return Nan::ThrowError("pipe() failed."); + Nan::ThrowError("pipe() failed."); + goto done; } posix_spawn_file_actions_t acts; @@ -263,8 +265,6 @@ NAN_METHOD(PtyFork) { pid_t pid; auto error = posix_spawn(&pid, helper_path, &acts, &attrs, argv, env); - posix_spawn_file_actions_destroy(&acts); - posix_spawnattr_destroy(&attrs); close(comms_pipe[1]); // reenable signals @@ -272,24 +272,21 @@ NAN_METHOD(PtyFork) { if (error) { perror("posix_spawn failed"); - return Nan::ThrowError("posix_spawn(3) failed."); + Nan::ThrowError("posix_spawn(3) failed."); + goto done; } auto bytes_read = read(comms_pipe[0], &error, sizeof(error)); close(comms_pipe[0]); - for (int i = 0; i < argl; i++) free(argv[i]); - delete[] argv; - for (int i = 0; i < envc; i++) free(env[i]); - delete[] env; - free(helper_path); - if (bytes_read && error) { - return Nan::ThrowError("exec error"); + Nan::ThrowError("exec error"); + goto done; } if (pty_nonblock(master) == -1) { - return Nan::ThrowError("Could not set master fd to nonblocking."); + Nan::ThrowError("Could not set master fd to nonblocking."); + goto done; } v8::Local obj = Nan::New(); @@ -314,7 +311,21 @@ NAN_METHOD(PtyFork) { uv_thread_create(&baton->tid, pty_waitpid, static_cast(baton)); - return info.GetReturnValue().Set(obj); + info.GetReturnValue().Set(obj); + +done: + posix_spawn_file_actions_destroy(&acts); + posix_spawnattr_destroy(&attrs); + + if (argv) { + for (int i = 0; i < argl; i++) free(argv[i]); + delete[] argv; + } + if (env) { + for (int i = 0; i < envc; i++) free(env[i]); + delete[] env; + } + free(helper_path); } NAN_METHOD(PtyOpen) { From 6fa43179dc2f1705a94d10f453f8b9d8f3bb81c6 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Tue, 10 Aug 2021 15:24:57 +0200 Subject: [PATCH 06/17] posix_spawn: handle TIOCSCTTY on solaris --- src/unix/spawn-helper.cc | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index 169273a6e..637070933 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -10,15 +10,6 @@ #include "comms.h" -/* environ for execvpe */ -/* node/src/node_child_process.cc */ -#if defined(__APPLE__) && !TARGET_OS_IPHONE - #include - #define environ (*_NSGetEnviron()) -#else - extern char **environ; -#endif - int main (int argc, char** argv) { sigset_t empty_set; sigemptyset(&empty_set); @@ -39,6 +30,12 @@ int main (int argc, char** argv) { if (ioctl(STDIN_FILENO, TIOCSCTTY, NULL) == -1) { _exit(1); } +#else + char *slave_path = ttyname(STDIN_FILENO); + // open implicit attaches a process to a terminal device if: + // - process has no controlling terminal yet + // - O_NOCTTY is not set + close(open(slave_path, O_RDWR)); #endif char *cwd = argv[0]; From 0e81e075d0d2cb0fd3c12c06e94c8d206b40fd28 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Tue, 10 Aug 2021 15:25:06 +0200 Subject: [PATCH 07/17] posix_spawn: lint --- src/unix/pty.cc | 89 +++++++++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index 9052b9490..d4bc3c461 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -262,57 +262,58 @@ NAN_METHOD(PtyFork) { posix_spawnattr_init(&attrs); posix_spawnattr_setflags(&attrs, POSIX_SPAWN_RESETIDS | POSIX_SPAWN_CLOEXEC_DEFAULT); - pid_t pid; - auto error = posix_spawn(&pid, helper_path, &acts, &attrs, argv, env); - - close(comms_pipe[1]); - - // reenable signals - pthread_sigmask(SIG_SETMASK, &oldmask, NULL); - - if (error) { - perror("posix_spawn failed"); - Nan::ThrowError("posix_spawn(3) failed."); - goto done; - } + { // suppresses "jump bypasses variable initialization" errors + pid_t pid; + auto error = posix_spawn(&pid, helper_path, &acts, &attrs, argv, env); - auto bytes_read = read(comms_pipe[0], &error, sizeof(error)); - close(comms_pipe[0]); - - if (bytes_read && error) { - Nan::ThrowError("exec error"); - goto done; - } - - if (pty_nonblock(master) == -1) { - Nan::ThrowError("Could not set master fd to nonblocking."); - goto done; - } + close(comms_pipe[1]); - v8::Local obj = Nan::New(); - Nan::Set(obj, - Nan::New("fd").ToLocalChecked(), - Nan::New(master)); - Nan::Set(obj, - Nan::New("pid").ToLocalChecked(), - Nan::New(pid)); - Nan::Set(obj, - Nan::New("pty").ToLocalChecked(), - Nan::New(ptsname(master)).ToLocalChecked()); + // reenable signals + pthread_sigmask(SIG_SETMASK, &oldmask, NULL); - pty_baton *baton = new pty_baton(); - baton->exit_code = 0; - baton->signal_code = 0; - baton->cb.Reset(v8::Local::Cast(info[9])); - baton->pid = pid; - baton->async.data = baton; + if (error) { + perror("posix_spawn failed"); + Nan::ThrowError("posix_spawn(3) failed."); + goto done; + } - uv_async_init(uv_default_loop(), &baton->async, pty_after_waitpid); + auto bytes_read = read(comms_pipe[0], &error, sizeof(error)); + close(comms_pipe[0]); - uv_thread_create(&baton->tid, pty_waitpid, static_cast(baton)); + if (bytes_read && error) { + Nan::ThrowError("exec error"); + goto done; + } - info.GetReturnValue().Set(obj); + if (pty_nonblock(master) == -1) { + Nan::ThrowError("Could not set master fd to nonblocking."); + goto done; + } + v8::Local obj = Nan::New(); + Nan::Set(obj, + Nan::New("fd").ToLocalChecked(), + Nan::New(master)); + Nan::Set(obj, + Nan::New("pid").ToLocalChecked(), + Nan::New(pid)); + Nan::Set(obj, + Nan::New("pty").ToLocalChecked(), + Nan::New(ptsname(master)).ToLocalChecked()); + + pty_baton *baton = new pty_baton(); + baton->exit_code = 0; + baton->signal_code = 0; + baton->cb.Reset(v8::Local::Cast(info[9])); + baton->pid = pid; + baton->async.data = baton; + + uv_async_init(uv_default_loop(), &baton->async, pty_after_waitpid); + + uv_thread_create(&baton->tid, pty_waitpid, static_cast(baton)); + + info.GetReturnValue().Set(obj); + } done: posix_spawn_file_actions_destroy(&acts); posix_spawnattr_destroy(&attrs); From 52db84bc8ffc23c1e9225669836cc48dba4e9db6 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Tue, 10 Aug 2021 15:27:22 +0200 Subject: [PATCH 08/17] headers cleanup --- src/unix/spawn-helper.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index 637070933..da0bf6820 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -1,10 +1,8 @@ #include #include -#include #include #include #include -#include #include #include From aa5a05d6121c229d2fa9fe2f150910bf384bd4eb Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Wed, 11 Aug 2021 00:34:20 +0200 Subject: [PATCH 09/17] posix_spawn: build fixes --- binding.gyp | 2 -- src/unix/spawn-helper.cc | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/binding.gyp b/binding.gyp index 9d548d9c7..48b16780e 100644 --- a/binding.gyp +++ b/binding.gyp @@ -53,7 +53,6 @@ 'cflags': ['-Wall'], 'sources': [ 'src/unix/spawn-helper.cc', - 'src/unix/comms.cc' ], 'libraries': [ '-lpthread' @@ -66,7 +65,6 @@ ], 'sources': [ 'src/unix/pty.cc', - 'src/unix/comms.cc' ], 'libraries': [ '-lutil' diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index da0bf6820..c638f20cc 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include "comms.h" From 4cd52e3a8e904c0305d662d2ece995d0d481a1c2 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Wed, 11 Aug 2021 00:34:53 +0200 Subject: [PATCH 10/17] posix_spawn: better error handling --- src/unix/comms.h | 4 ++++ src/unix/pty.cc | 21 ++++++++++++++++++--- src/unix/spawn-helper.cc | 17 ++++++++++------- src/unixTerminal.test.ts | 18 ++++++++++++++++++ 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/unix/comms.h b/src/unix/comms.h index 8d0cf543f..b95554370 100644 --- a/src/unix/comms.h +++ b/src/unix/comms.h @@ -1 +1,5 @@ #define COMM_PIPE_FD (STDERR_FILENO + 1) +#define COMM_ERR_EXEC 1 +#define COMM_ERR_CHDIR 2 +#define COMM_ERR_SETUID 3 +#define COMM_ERR_SETGID 4 diff --git a/src/unix/pty.cc b/src/unix/pty.cc index d4bc3c461..ffccc78fa 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -121,6 +121,12 @@ pty_after_waitpid(uv_async_t *); static void pty_after_close(uv_handle_t *); +static void throw_for_errno(const char* message, int _errno) { + Nan::ThrowError(( + message + std::string(strerror(_errno)) + ).c_str()); +} + NAN_METHOD(PtyFork) { Nan::HandleScope scope; @@ -277,11 +283,20 @@ NAN_METHOD(PtyFork) { goto done; } - auto bytes_read = read(comms_pipe[0], &error, sizeof(error)); + int helper_error[2]; + auto bytes_read = read(comms_pipe[0], &helper_error, sizeof(helper_error)); close(comms_pipe[0]); - if (bytes_read && error) { - Nan::ThrowError("exec error"); + if (bytes_read == sizeof(helper_error)) { + if (helper_error[0] == COMM_ERR_EXEC) { + throw_for_errno("exec() failed: ", helper_error[1]); + } else if (helper_error[0] == COMM_ERR_CHDIR) { + throw_for_errno("chdir() failed: ", helper_error[1]); + } else if (helper_error[0] == COMM_ERR_SETUID) { + throw_for_errno("setuid() failed: ", helper_error[1]); + } else if (helper_error[0] == COMM_ERR_SETGID) { + throw_for_errno("setgid() failed: ", helper_error[1]); + } goto done; } diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index c638f20cc..7feb9a49f 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -9,6 +9,12 @@ #include "comms.h" +void bail (int type, int code) { + int buf[2] = { type, code }; + (void)! write(COMM_PIPE_FD, &buf, sizeof(buf)); + _exit(1); +} + int main (int argc, char** argv) { sigset_t empty_set; sigemptyset(&empty_set); @@ -46,19 +52,16 @@ int main (int argc, char** argv) { fcntl(COMM_PIPE_FD, F_SETFD, FD_CLOEXEC); if (strlen(cwd) && chdir(cwd) == -1) { - perror("chdir(2) failed."); - _exit(1); + bail(COMM_ERR_CHDIR, errno); } if (uid != -1 && setuid(uid) == -1) { - perror("setuid(2) failed."); - _exit(1); + bail(COMM_ERR_SETUID, errno); } if (gid != -1 && setgid(gid) == -1) { - perror("setgid(2) failed."); - _exit(1); + bail(COMM_ERR_SETGID, errno); } execvp(file, argv); - write(COMM_PIPE_FD, &errno, sizeof(errno)); + bail(COMM_ERR_EXEC, errno); return 1; } diff --git a/src/unixTerminal.test.ts b/src/unixTerminal.test.ts index 9fec50eeb..209fd648b 100644 --- a/src/unixTerminal.test.ts +++ b/src/unixTerminal.test.ts @@ -248,6 +248,24 @@ if (process.platform !== 'win32') { done(); } }); + it('should handle chdir() errors', (done) => { + try { + new UnixTerminal('/bin/echo', [], { cwd: '/nowhere' }); + done(new Error('should have failed')); + } catch (e) { + assert.equal(e.toString(), 'Error: chdir() failed: No such file or directory'); + done(); + } + }); + it('should handle setuid() errors', (done) => { + try { + new UnixTerminal('/bin/echo', [], { uid: 999999 }); + done(new Error('should have failed')); + } catch (e) { + assert.equal(e.toString(), 'Error: setuid() failed: Operation not permitted'); + done(); + } + }); }); }); } From 272cad70d0a2aa7c7ed81d2f2a388d3243027552 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Wed, 11 Aug 2021 00:35:01 +0200 Subject: [PATCH 11/17] posix_spawn: faster spawn --- src/unix/pty.cc | 6 +++++- src/unix/spawn-helper.cc | 8 -------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index ffccc78fa..ac757572f 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -75,6 +75,10 @@ #define POSIX_SPAWN_CLOEXEC_DEFAULT 0 #endif +#ifndef POSIX_SPAWN_USEVFORK + #define POSIX_SPAWN_USEVFORK 0 +#endif + /** * Structs */ @@ -266,7 +270,7 @@ NAN_METHOD(PtyFork) { posix_spawnattr_t attrs; posix_spawnattr_init(&attrs); - posix_spawnattr_setflags(&attrs, POSIX_SPAWN_RESETIDS | POSIX_SPAWN_CLOEXEC_DEFAULT); + posix_spawnattr_setflags(&attrs, POSIX_SPAWN_CLOEXEC_DEFAULT | POSIX_SPAWN_USEVFORK); { // suppresses "jump bypasses variable initialization" errors pid_t pid; diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index 7feb9a49f..57c898d57 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -20,14 +20,6 @@ int main (int argc, char** argv) { sigemptyset(&empty_set); pthread_sigmask(SIG_SETMASK, &empty_set, nullptr); - struct rlimit rlim_ofile; - getrlimit(RLIMIT_NOFILE, &rlim_ofile); - for (rlim_t fd = STDERR_FILENO + 1; fd < rlim_ofile.rlim_cur; fd++) { - if (fd != COMM_PIPE_FD) { - close(fd); - } - } - setsid(); #if defined(TIOCSCTTY) From dea39fa2df9492577c459bd9fc3f340c570d1d10 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Wed, 11 Aug 2021 17:35:22 +0200 Subject: [PATCH 12/17] posix_spawn: optionally close FDs --- src/interfaces.ts | 1 + src/native.d.ts | 2 +- src/unix/pty.cc | 35 ++++++++++++++++++++++++----------- src/unix/spawn-helper.cc | 14 ++++++++++++-- src/unixTerminal.ts | 3 ++- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/interfaces.ts b/src/interfaces.ts index 3df3ccbcf..f14ffde71 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -109,6 +109,7 @@ interface IBasePtyForkOptions { export interface IPtyForkOptions extends IBasePtyForkOptions { uid?: number; gid?: number; + closeFDs?: boolean; } export interface IWindowsPtyForkOptions extends IBasePtyForkOptions { diff --git a/src/native.d.ts b/src/native.d.ts index 05fd1b209..4445ad49f 100644 --- a/src/native.d.ts +++ b/src/native.d.ts @@ -18,7 +18,7 @@ interface IWinptyNative { } interface IUnixNative { - fork(file: string, args: string[], parsedEnv: string[], cwd: string, cols: number, rows: number, uid: number, gid: number, useUtf8: boolean, onExitCallback: (code: number, signal: number) => void, helperPath: string): IUnixProcess; + fork(file: string, args: string[], parsedEnv: string[], cwd: string, cols: number, rows: number, uid: number, gid: number, closeFDs: boolean, useUtf8: boolean, onExitCallback: (code: number, signal: number) => void, helperPath: string): IUnixProcess; open(cols: number, rows: number): IUnixOpenProcess; process(fd: number, pty: string): string; resize(fd: number, cols: number, rows: number): void; diff --git a/src/unix/pty.cc b/src/unix/pty.cc index ac757572f..d4f17f871 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -71,7 +71,10 @@ #define NSIG 32 #endif -#ifndef POSIX_SPAWN_CLOEXEC_DEFAULT +#ifdef POSIX_SPAWN_CLOEXEC_DEFAULT + #define HAVE_POSIX_SPAWN_CLOEXEC_DEFAULT 1 +#else + #define HAVE_POSIX_SPAWN_CLOEXEC_DEFAULT 0 #define POSIX_SPAWN_CLOEXEC_DEFAULT 0 #endif @@ -134,7 +137,7 @@ static void throw_for_errno(const char* message, int _errno) { NAN_METHOD(PtyFork) { Nan::HandleScope scope; - if (info.Length() != 11 || + if (info.Length() != 12 || !info[0]->IsString() || !info[1]->IsArray() || !info[2]->IsArray() || @@ -144,10 +147,11 @@ NAN_METHOD(PtyFork) { !info[6]->IsNumber() || !info[7]->IsNumber() || !info[8]->IsBoolean() || - !info[9]->IsFunction() || - !info[10]->IsString()) { + !info[9]->IsBoolean() || + !info[10]->IsFunction() || + !info[11]->IsString()) { return Nan::ThrowError( - "Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, utf8, onexit, helperPath)"); + "Usage: pty.fork(file, args, env, cwd, cols, rows, uid, gid, closeFDs, utf8, onexit, helperPath)"); } // file @@ -170,17 +174,22 @@ NAN_METHOD(PtyFork) { int uid = info[6]->IntegerValue(Nan::GetCurrentContext()).FromJust(); int gid = info[7]->IntegerValue(Nan::GetCurrentContext()).FromJust(); + // closeFDs + bool closeFDs = Nan::To(info[8]).FromJust(); + bool explicitlyCloseFDs = closeFDs && !HAVE_POSIX_SPAWN_CLOEXEC_DEFAULT; + // args v8::Local argv_ = v8::Local::Cast(info[1]); - const int EXTRA_ARGS = 4; + const int EXTRA_ARGS = 5; int argc = argv_->Length(); int argl = argc + EXTRA_ARGS + 1; char **argv = new char*[argl]; argv[0] = strdup(*cwd_); argv[1] = strdup(std::to_string(uid).c_str()); argv[2] = strdup(std::to_string(gid).c_str()); - argv[3] = strdup(*file); + argv[3] = strdup(explicitlyCloseFDs ? "1": "0"); + argv[4] = strdup(*file); argv[argl - 1] = NULL; for (int i = 0; i < argc; i++) { Nan::Utf8String arg(Nan::Get(argv_, i).ToLocalChecked()); @@ -198,7 +207,7 @@ NAN_METHOD(PtyFork) { struct termios t = termios(); struct termios *term = &t; term->c_iflag = ICRNL | IXON | IXANY | IMAXBEL | BRKINT; - if (Nan::To(info[8]).FromJust()) { + if (Nan::To(info[9]).FromJust()) { #if defined(IUTF8) term->c_iflag |= IUTF8; #endif @@ -233,10 +242,11 @@ NAN_METHOD(PtyFork) { cfsetospeed(term, B38400); // helperPath - Nan::Utf8String helper_path_(info[10]); + Nan::Utf8String helper_path_(info[11]); char *helper_path = strdup(*helper_path_); sigset_t newmask, oldmask; + int flags = POSIX_SPAWN_USEVFORK; // temporarily block all signals // this is needed due to a race condition in openpty @@ -270,7 +280,10 @@ NAN_METHOD(PtyFork) { posix_spawnattr_t attrs; posix_spawnattr_init(&attrs); - posix_spawnattr_setflags(&attrs, POSIX_SPAWN_CLOEXEC_DEFAULT | POSIX_SPAWN_USEVFORK); + if (closeFDs) { + flags |= POSIX_SPAWN_CLOEXEC_DEFAULT; + } + posix_spawnattr_setflags(&attrs, flags); { // suppresses "jump bypasses variable initialization" errors pid_t pid; @@ -323,7 +336,7 @@ NAN_METHOD(PtyFork) { pty_baton *baton = new pty_baton(); baton->exit_code = 0; baton->signal_code = 0; - baton->cb.Reset(v8::Local::Cast(info[9])); + baton->cb.Reset(v8::Local::Cast(info[10])); baton->pid = pid; baton->async.data = baton; diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index 57c898d57..9ba87e823 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -38,8 +38,9 @@ int main (int argc, char** argv) { char *cwd = argv[0]; int uid = std::stoi(argv[1]); int gid = std::stoi(argv[2]); - char *file = argv[3]; - argv = &argv[3]; + bool closeFDs = std::stoi(argv[3]); + char *file = argv[4]; + argv = &argv[4]; fcntl(COMM_PIPE_FD, F_SETFD, FD_CLOEXEC); @@ -52,6 +53,15 @@ int main (int argc, char** argv) { if (gid != -1 && setgid(gid) == -1) { bail(COMM_ERR_SETGID, errno); } + if (closeFDs) { + struct rlimit rlim_ofile; + getrlimit(RLIMIT_NOFILE, &rlim_ofile); + for (rlim_t fd = STDERR_FILENO + 1; fd < rlim_ofile.rlim_cur; fd++) { + if (fd != COMM_PIPE_FD) { + close(fd); + } + } + } execvp(file, argv); bail(COMM_ERR_EXEC, errno); diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index 576270af9..fc5216b9d 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -67,6 +67,7 @@ export class UnixTerminal extends Terminal { this._rows = opt.rows || DEFAULT_ROWS; const uid = opt.uid || -1; const gid = opt.gid || -1; + const closeFDs = opt.closeFDs || false; const env: IProcessEnv = assign({}, opt.env); if (opt.env === process.env) { @@ -109,7 +110,7 @@ export class UnixTerminal extends Terminal { }; // fork - const term = pty.fork(file, args, parsedEnv, cwd, this._cols, this._rows, uid, gid, (encoding === 'utf8'), onexit, helperPath); + const term = pty.fork(file, args, parsedEnv, cwd, this._cols, this._rows, uid, gid, (encoding === 'utf8'), closeFDs, onexit, helperPath); this._socket = new PipeSocket(term.fd); if (encoding !== null) { From 1ffd01b7e15f0113dd5aa3ccb77e90ece7e6678f Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Wed, 11 Aug 2021 17:40:07 +0200 Subject: [PATCH 13/17] posix_spawn: document closeFDs --- typings/node-pty.d.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/typings/node-pty.d.ts b/typings/node-pty.d.ts index ccab4a0c2..9dd3286cc 100644 --- a/typings/node-pty.d.ts +++ b/typings/node-pty.d.ts @@ -78,11 +78,17 @@ declare module 'node-pty' { export interface IPtyForkOptions extends IBasePtyForkOptions { /** - * Security warning: use this option with great caution, as opened file descriptors - * with higher privileges might leak to the child program. + * Security warning: use this option with great caution unless `closeFDs` is also set, + * as opened file descriptors with higher privileges might leak to the child program. */ uid?: number; gid?: number; + + /** + * Close all open file after spawning the child process (UNIX only). + * Carries a performance penalty on Linux. + */ + closeFDs?: boolean; } export interface IWindowsPtyForkOptions extends IBasePtyForkOptions { From b3753df0954469662107a80758942ad900efd3e9 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Sun, 15 Aug 2021 23:41:17 +0200 Subject: [PATCH 14/17] posix_spawn: use the same MACOSX_DEPLOYMENT_TARGET as the pty library --- binding.gyp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/binding.gyp b/binding.gyp index 48b16780e..b2c262dbf 100644 --- a/binding.gyp +++ b/binding.gyp @@ -54,9 +54,13 @@ 'sources': [ 'src/unix/spawn-helper.cc', ], - 'libraries': [ - '-lpthread' - ], + 'conditions': [ + ['OS=="mac"', { + "xcode_settings": { + "MACOSX_DEPLOYMENT_TARGET":"10.7" + } + }] + ] }, { 'target_name': 'pty', From 300fea85df47f0db6c99965fc3f2edf4745173b8 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Mon, 16 Aug 2021 13:39:00 +0200 Subject: [PATCH 15/17] include posix_spawn errno in exception --- src/unix/pty.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/unix/pty.cc b/src/unix/pty.cc index d4f17f871..2c277f7bb 100644 --- a/src/unix/pty.cc +++ b/src/unix/pty.cc @@ -295,8 +295,7 @@ NAN_METHOD(PtyFork) { pthread_sigmask(SIG_SETMASK, &oldmask, NULL); if (error) { - perror("posix_spawn failed"); - Nan::ThrowError("posix_spawn(3) failed."); + throw_for_errno("posix_spawn failed: ", error); goto done; } From 62d67e3f1b95c0ae663a60b5101dbd928b0812a2 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Mon, 16 Aug 2021 13:39:11 +0200 Subject: [PATCH 16/17] posix_spawn: handle being packaged in an ASAR --- src/unixTerminal.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/unixTerminal.ts b/src/unixTerminal.ts index fc5216b9d..3d214ce43 100644 --- a/src/unixTerminal.ts +++ b/src/unixTerminal.ts @@ -27,6 +27,8 @@ try { } helperPath = path.resolve(__dirname, helperPath); +helperPath = helperPath.replace('app.asar', 'app.asar.unpacked'); +helperPath = helperPath.replace('node_modules.asar', 'node_modules.asar.unpacked'); const DEFAULT_FILE = 'sh'; const DEFAULT_NAME = 'xterm'; From ad49e66354e1df5c479962c19be003a09af9fd16 Mon Sep 17 00:00:00 2001 From: Eugene Pankov Date: Sun, 22 Aug 2021 17:29:37 +0200 Subject: [PATCH 17/17] posix_spawn: added missing include --- src/unix/spawn-helper.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/unix/spawn-helper.cc b/src/unix/spawn-helper.cc index 9ba87e823..35358bfcd 100644 --- a/src/unix/spawn-helper.cc +++ b/src/unix/spawn-helper.cc @@ -1,3 +1,4 @@ +#include #include #include #include