Skip to content

Commit

Permalink
src: allow CLI args in env with NODE_OPTIONS
Browse files Browse the repository at this point in the history
Not all CLI options are supported, those that are problematic from a
security or implementation point of view are disallowed, as are ones
that are inappropriate (for example, -e, -p, --i), or that only make
sense when changed with code changes (such as options that change the
javascript syntax or add new APIs).

Backport-PR-URL: #12677
PR-URL: #12028
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
  • Loading branch information
sam-github authored and MylesBorins committed Oct 25, 2017
1 parent 8f42148 commit dd6ea89
Show file tree
Hide file tree
Showing 6 changed files with 260 additions and 39 deletions.
8 changes: 8 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,11 @@ parser.add_option('--without-ssl',
dest='without_ssl',
help='build without SSL (disables crypto, https, inspector, etc.)')

parser.add_option('--without-node-options',
action='store_true',
dest='without_node_options',
help='build without NODE_OPTIONS support')

parser.add_option('--xcode',
action='store_true',
dest='use_xcode',
Expand Down Expand Up @@ -975,6 +980,9 @@ def configure_openssl(o):
o['variables']['openssl_no_asm'] = 1 if options.openssl_no_asm else 0
if options.use_openssl_ca_store:
o['defines'] += ['NODE_OPENSSL_CERT_STORE']
o['variables']['node_without_node_options'] = b(options.without_node_options)
if options.without_node_options:
o['defines'] += ['NODE_WITHOUT_NODE_OPTIONS']
if options.openssl_fips:
o['variables']['openssl_fips'] = options.openssl_fips
fips_dir = os.path.join(root_dir, 'deps', 'openssl', 'fips')
Expand Down
35 changes: 35 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,41 @@ added: v6.11.0

When set to `1`, process warnings are silenced.

### `NODE_OPTIONS=options...`
<!-- YAML
added: REPLACEME
-->

`options...` are interpreted as if they had been specified on the command line
before the actual command line (so they can be overriden). Node will exit with
an error if an option that is not allowed in the environment is used, such as
`-p` or a script file.

Node options that are allowed are:
- `--enable-fips`
- `--force-fips`
- `--icu-data-dir`
- `--no-deprecation`
- `--no-warnings`
- `--openssl-config`
- `--prof-process`
- `--redirect-warnings`
- `--require`, `-r`
- `--throw-deprecation`
- `--trace-deprecation`
- `--trace-events-enabled`
- `--trace-sync-io`
- `--trace-warnings`
- `--track-heap-objects`
- `--use-bundled-ca`
- `--use-openssl-ca`
- `--v8-pool-size`
- `--zero-fill-buffers`

V8 options that are allowed are:
- `--max_old_space_size`


### `NODE_REPL_HISTORY=file`
<!-- YAML
added: v3.0.0
Expand Down
7 changes: 7 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ with small\-icu support.
.BR NODE_NO_WARNINGS =\fI1\fR
When set to \fI1\fR, process warnings are silenced.

.TP
.BR NODE_OPTIONS =\fIoptions...\fR
\fBoptions...\fR are interpreted as if they had been specified on the command
line before the actual command line (so they can be overriden). Node will exit
with an error if an option that is not allowed in the environment is used, such
as \fB-p\fR or a script file.

.TP
.BR NODE_REPL_HISTORY =\fIfile\fR
Path to the file used to store the persistent REPL history. The default path
Expand Down
169 changes: 132 additions & 37 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3759,6 +3759,9 @@ static void PrintHelp() {
#endif
#endif
"NODE_NO_WARNINGS set to 1 to silence process warnings\n"
#if !defined(NODE_WITHOUT_NODE_OPTIONS)
"NODE_OPTIONS set CLI options in the environment\n"
#endif
#ifdef _WIN32
"NODE_PATH ';'-separated list of directories\n"
#else
Expand All @@ -3773,6 +3776,50 @@ static void PrintHelp() {
}


static void CheckIfAllowedInEnv(const char* exe, bool is_env,
const char* arg) {
if (!is_env)
return;

// Find the arg prefix when its --some_arg=val
const char* eq = strchr(arg, '=');
size_t arglen = eq ? eq - arg : strlen(arg);

static const char* whitelist[] = {
// Node options
"-r", "--require",
"--no-deprecation",
"--no-warnings",
"--trace-warnings",
"--redirect-warnings",
"--trace-deprecation",
"--trace-sync-io",
"--track-heap-objects",
"--throw-deprecation",
"--zero-fill-buffers",
"--v8-pool-size",
"--use-openssl-ca",
"--use-bundled-ca",
"--enable-fips",
"--force-fips",
"--openssl-config",
"--icu-data-dir",

// V8 options
"--max_old_space_size",
};

for (unsigned i = 0; i < arraysize(whitelist); i++) {
const char* allowed = whitelist[i];
if (strlen(allowed) == arglen && strncmp(allowed, arg, arglen) == 0)
return;
}

fprintf(stderr, "%s: %s is not allowed in NODE_OPTIONS\n", exe, arg);
exit(9);
}


// Parse command line arguments.
//
// argv is modified in place. exec_argv and v8_argv are out arguments that
Expand All @@ -3789,7 +3836,8 @@ static void ParseArgs(int* argc,
int* exec_argc,
const char*** exec_argv,
int* v8_argc,
const char*** v8_argv) {
const char*** v8_argv,
bool is_env) {
const unsigned int nargs = static_cast<unsigned int>(*argc);
const char** new_exec_argv = new const char*[nargs];
const char** new_v8_argv = new const char*[nargs];
Expand All @@ -3814,6 +3862,8 @@ static void ParseArgs(int* argc,
const char* const arg = argv[index];
unsigned int args_consumed = 1;

CheckIfAllowedInEnv(argv[0], is_env, arg);

if (ParseDebugOpt(arg)) {
// Done, consumed by ParseDebugOpt().
} else if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) {
Expand Down Expand Up @@ -3934,6 +3984,13 @@ static void ParseArgs(int* argc,

// Copy remaining arguments.
const unsigned int args_left = nargs - index;

if (is_env && args_left) {
fprintf(stderr, "%s: %s is not supported in NODE_OPTIONS\n",
argv[0], argv[index]);
exit(9);
}

memcpy(new_argv + new_argc, argv + index, args_left * sizeof(*argv));
new_argc += args_left;

Expand Down Expand Up @@ -4367,6 +4424,54 @@ inline void PlatformInit() {
}


void ProcessArgv(int* argc,
const char** argv,
int* exec_argc,
const char*** exec_argv,
bool is_env = false) {
// Parse a few arguments which are specific to Node.
int v8_argc;
const char** v8_argv;
ParseArgs(argc, argv, exec_argc, exec_argv, &v8_argc, &v8_argv, is_env);

// TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler
// manually? That would give us a little more control over its runtime
// behavior but it could also interfere with the user's intentions in ways
// we fail to anticipate. Dillema.
for (int i = 1; i < v8_argc; ++i) {
if (strncmp(v8_argv[i], "--prof", sizeof("--prof") - 1) == 0) {
v8_is_profiling = true;
break;
}
}

#ifdef __POSIX__
// Block SIGPROF signals when sleeping in epoll_wait/kevent/etc. Avoids the
// performance penalty of frequent EINTR wakeups when the profiler is running.
// Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
if (v8_is_profiling) {
uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
}
#endif

// The const_cast doesn't violate conceptual const-ness. V8 doesn't modify
// the argv array or the elements it points to.
if (v8_argc > 1)
V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);

// Anything that's still in v8_argv is not a V8 or a node option.
for (int i = 1; i < v8_argc; i++) {
fprintf(stderr, "%s: bad option: %s\n", argv[0], v8_argv[i]);
}
delete[] v8_argv;
v8_argv = nullptr;

if (v8_argc > 1) {
exit(9);
}
}


void Init(int* argc,
const char** argv,
int* exec_argc,
Expand Down Expand Up @@ -4397,31 +4502,36 @@ void Init(int* argc,
if (config_warning_file.empty())
SafeGetenv("NODE_REDIRECT_WARNINGS", &config_warning_file);

// Parse a few arguments which are specific to Node.
int v8_argc;
const char** v8_argv;
ParseArgs(argc, argv, exec_argc, exec_argv, &v8_argc, &v8_argv);

// TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler
// manually? That would give us a little more control over its runtime
// behavior but it could also interfere with the user's intentions in ways
// we fail to anticipate. Dillema.
for (int i = 1; i < v8_argc; ++i) {
if (strncmp(v8_argv[i], "--prof", sizeof("--prof") - 1) == 0) {
v8_is_profiling = true;
break;
#if !defined(NODE_WITHOUT_NODE_OPTIONS)
std::string node_options;
if (SafeGetenv("NODE_OPTIONS", &node_options)) {
// Smallest tokens are 2-chars (a not space and a space), plus 2 extra
// pointers, for the prepended executable name, and appended NULL pointer.
size_t max_len = 2 + (node_options.length() + 1) / 2;
const char** argv_from_env = new const char*[max_len];
int argc_from_env = 0;
// [0] is expected to be the program name, fill it in from the real argv.
argv_from_env[argc_from_env++] = argv[0];

char* cstr = strdup(node_options.c_str());
char* initptr = cstr;
char* token;
while ((token = strtok(initptr, " "))) { // NOLINT(runtime/threadsafe_fn)
initptr = nullptr;
argv_from_env[argc_from_env++] = token;
}
}

#ifdef __POSIX__
// Block SIGPROF signals when sleeping in epoll_wait/kevent/etc. Avoids the
// performance penalty of frequent EINTR wakeups when the profiler is running.
// Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
if (v8_is_profiling) {
uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
argv_from_env[argc_from_env] = nullptr;
int exec_argc_;
const char** exec_argv_ = nullptr;
ProcessArgv(&argc_from_env, argv_from_env, &exec_argc_, &exec_argv_, true);
delete[] exec_argv_;
delete[] argv_from_env;
free(cstr);
}
#endif

ProcessArgv(argc, argv, exec_argc, exec_argv);

#if defined(NODE_HAVE_I18N_SUPPORT)
// If the parameter isn't given, use the env variable.
if (icu_data_dir.empty())
Expand All @@ -4433,21 +4543,6 @@ void Init(int* argc,
"(check NODE_ICU_DATA or --icu-data-dir parameters)\n");
}
#endif
// The const_cast doesn't violate conceptual const-ness. V8 doesn't modify
// the argv array or the elements it points to.
if (v8_argc > 1)
V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);

// Anything that's still in v8_argv is not a V8 or a node option.
for (int i = 1; i < v8_argc; i++) {
fprintf(stderr, "%s: bad option: %s\n", argv[0], v8_argv[i]);
}
delete[] v8_argv;
v8_argv = nullptr;

if (v8_argc > 1) {
exit(9);
}

// Unconditionally force typed arrays to allocate outside the v8 heap. This
// is to prevent memory pointers from being moved around that are returned by
Expand Down
74 changes: 74 additions & 0 deletions test/parallel/test-cli-node-options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
'use strict';
const common = require('../common');
if (process.config.variables.node_without_node_options)
return common.skip('missing NODE_OPTIONS support');

// Test options specified by env variable.

const assert = require('assert');
const exec = require('child_process').execFile;

disallow('--version');
disallow('-v');
disallow('--help');
disallow('-h');
disallow('--eval');
disallow('-e');
disallow('--print');
disallow('-p');
disallow('-pe');
disallow('--check');
disallow('-c');
disallow('--interactive');
disallow('-i');
disallow('--v8-options');
disallow('--');

function disallow(opt) {
const options = {env: {NODE_OPTIONS: opt}};
exec(process.execPath, options, common.mustCall(function(err) {
const message = err.message.split(/\r?\n/)[1];
const expect = process.execPath + ': ' + opt +
' is not allowed in NODE_OPTIONS';

assert.strictEqual(err.code, 9);
assert.strictEqual(message, expect);
}));
}

const printA = require.resolve('../fixtures/printA.js');

expect('-r ' + printA, 'A\nB\n');
expect('--no-deprecation', 'B\n');
expect('--no-warnings', 'B\n');
expect('--trace-warnings', 'B\n');
expect('--trace-deprecation', 'B\n');
expect('--trace-sync-io', 'B\n');
expect('--track-heap-objects', 'B\n');
expect('--throw-deprecation', 'B\n');
expect('--zero-fill-buffers', 'B\n');
expect('--v8-pool-size=10', 'B\n');
expect('--use-openssl-ca', 'B\n');
expect('--use-bundled-ca', 'B\n');
expect('--openssl-config=_ossl_cfg', 'B\n');
expect('--icu-data-dir=_d', 'B\n');

// V8 options
expect('--max_old_space_size=0', 'B\n');

function expect(opt, want) {
const printB = require.resolve('../fixtures/printB.js');
const argv = [printB];
const opts = {
env: {NODE_OPTIONS: opt},
maxBuffer: 1000000000,
};
exec(process.execPath, argv, opts, common.mustCall(function(err, stdout) {
assert.ifError(err);
if (!RegExp(want).test(stdout)) {
console.error('For %j, failed to find %j in: <\n%s\n>',
opt, expect, stdout);
assert(false, 'Expected ' + expect);
}
}));
}
Loading

0 comments on commit dd6ea89

Please sign in to comment.