Skip to content

Commit 2871340

Browse files
sam-githubMylesBorins
authored andcommitted
src: allow CLI args in env with NODE_OPTIONS
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 <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Gibson Fahnestock <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
1 parent 69c1329 commit 2871340

File tree

6 files changed

+260
-39
lines changed

6 files changed

+260
-39
lines changed

configure

+8
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,11 @@ parser.add_option('--without-ssl',
444444
dest='without_ssl',
445445
help='build without SSL (disables crypto, https, inspector, etc.)')
446446

447+
parser.add_option('--without-node-options',
448+
action='store_true',
449+
dest='without_node_options',
450+
help='build without NODE_OPTIONS support')
451+
447452
parser.add_option('--xcode',
448453
action='store_true',
449454
dest='use_xcode',
@@ -975,6 +980,9 @@ def configure_openssl(o):
975980
o['variables']['openssl_no_asm'] = 1 if options.openssl_no_asm else 0
976981
if options.use_openssl_ca_store:
977982
o['defines'] += ['NODE_OPENSSL_CERT_STORE']
983+
o['variables']['node_without_node_options'] = b(options.without_node_options)
984+
if options.without_node_options:
985+
o['defines'] += ['NODE_WITHOUT_NODE_OPTIONS']
978986
if options.openssl_fips:
979987
o['variables']['openssl_fips'] = options.openssl_fips
980988
fips_dir = os.path.join(root_dir, 'deps', 'openssl', 'fips')

doc/api/cli.md

+35
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,41 @@ added: v6.11.0
332332

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

335+
### `NODE_OPTIONS=options...`
336+
<!-- YAML
337+
added: REPLACEME
338+
-->
339+
340+
`options...` are interpreted as if they had been specified on the command line
341+
before the actual command line (so they can be overriden). Node will exit with
342+
an error if an option that is not allowed in the environment is used, such as
343+
`-p` or a script file.
344+
345+
Node options that are allowed are:
346+
- `--enable-fips`
347+
- `--force-fips`
348+
- `--icu-data-dir`
349+
- `--no-deprecation`
350+
- `--no-warnings`
351+
- `--openssl-config`
352+
- `--prof-process`
353+
- `--redirect-warnings`
354+
- `--require`, `-r`
355+
- `--throw-deprecation`
356+
- `--trace-deprecation`
357+
- `--trace-events-enabled`
358+
- `--trace-sync-io`
359+
- `--trace-warnings`
360+
- `--track-heap-objects`
361+
- `--use-bundled-ca`
362+
- `--use-openssl-ca`
363+
- `--v8-pool-size`
364+
- `--zero-fill-buffers`
365+
366+
V8 options that are allowed are:
367+
- `--max_old_space_size`
368+
369+
335370
### `NODE_REPL_HISTORY=file`
336371
<!-- YAML
337372
added: v3.0.0

doc/node.1

+7
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,13 @@ with small\-icu support.
226226
.BR NODE_NO_WARNINGS =\fI1\fR
227227
When set to \fI1\fR, process warnings are silenced.
228228

229+
.TP
230+
.BR NODE_OPTIONS =\fIoptions...\fR
231+
\fBoptions...\fR are interpreted as if they had been specified on the command
232+
line before the actual command line (so they can be overriden). Node will exit
233+
with an error if an option that is not allowed in the environment is used, such
234+
as \fB-p\fR or a script file.
235+
229236
.TP
230237
.BR NODE_REPL_HISTORY =\fIfile\fR
231238
Path to the file used to store the persistent REPL history. The default path

src/node.cc

+132-37
Original file line numberDiff line numberDiff line change
@@ -3759,6 +3759,9 @@ static void PrintHelp() {
37593759
#endif
37603760
#endif
37613761
"NODE_NO_WARNINGS set to 1 to silence process warnings\n"
3762+
#if !defined(NODE_WITHOUT_NODE_OPTIONS)
3763+
"NODE_OPTIONS set CLI options in the environment\n"
3764+
#endif
37623765
#ifdef _WIN32
37633766
"NODE_PATH ';'-separated list of directories\n"
37643767
#else
@@ -3773,6 +3776,50 @@ static void PrintHelp() {
37733776
}
37743777

37753778

3779+
static void CheckIfAllowedInEnv(const char* exe, bool is_env,
3780+
const char* arg) {
3781+
if (!is_env)
3782+
return;
3783+
3784+
// Find the arg prefix when its --some_arg=val
3785+
const char* eq = strchr(arg, '=');
3786+
size_t arglen = eq ? eq - arg : strlen(arg);
3787+
3788+
static const char* whitelist[] = {
3789+
// Node options
3790+
"-r", "--require",
3791+
"--no-deprecation",
3792+
"--no-warnings",
3793+
"--trace-warnings",
3794+
"--redirect-warnings",
3795+
"--trace-deprecation",
3796+
"--trace-sync-io",
3797+
"--track-heap-objects",
3798+
"--throw-deprecation",
3799+
"--zero-fill-buffers",
3800+
"--v8-pool-size",
3801+
"--use-openssl-ca",
3802+
"--use-bundled-ca",
3803+
"--enable-fips",
3804+
"--force-fips",
3805+
"--openssl-config",
3806+
"--icu-data-dir",
3807+
3808+
// V8 options
3809+
"--max_old_space_size",
3810+
};
3811+
3812+
for (unsigned i = 0; i < arraysize(whitelist); i++) {
3813+
const char* allowed = whitelist[i];
3814+
if (strlen(allowed) == arglen && strncmp(allowed, arg, arglen) == 0)
3815+
return;
3816+
}
3817+
3818+
fprintf(stderr, "%s: %s is not allowed in NODE_OPTIONS\n", exe, arg);
3819+
exit(9);
3820+
}
3821+
3822+
37763823
// Parse command line arguments.
37773824
//
37783825
// argv is modified in place. exec_argv and v8_argv are out arguments that
@@ -3789,7 +3836,8 @@ static void ParseArgs(int* argc,
37893836
int* exec_argc,
37903837
const char*** exec_argv,
37913838
int* v8_argc,
3792-
const char*** v8_argv) {
3839+
const char*** v8_argv,
3840+
bool is_env) {
37933841
const unsigned int nargs = static_cast<unsigned int>(*argc);
37943842
const char** new_exec_argv = new const char*[nargs];
37953843
const char** new_v8_argv = new const char*[nargs];
@@ -3814,6 +3862,8 @@ static void ParseArgs(int* argc,
38143862
const char* const arg = argv[index];
38153863
unsigned int args_consumed = 1;
38163864

3865+
CheckIfAllowedInEnv(argv[0], is_env, arg);
3866+
38173867
if (ParseDebugOpt(arg)) {
38183868
// Done, consumed by ParseDebugOpt().
38193869
} else if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) {
@@ -3934,6 +3984,13 @@ static void ParseArgs(int* argc,
39343984

39353985
// Copy remaining arguments.
39363986
const unsigned int args_left = nargs - index;
3987+
3988+
if (is_env && args_left) {
3989+
fprintf(stderr, "%s: %s is not supported in NODE_OPTIONS\n",
3990+
argv[0], argv[index]);
3991+
exit(9);
3992+
}
3993+
39373994
memcpy(new_argv + new_argc, argv + index, args_left * sizeof(*argv));
39383995
new_argc += args_left;
39393996

@@ -4367,6 +4424,54 @@ inline void PlatformInit() {
43674424
}
43684425

43694426

4427+
void ProcessArgv(int* argc,
4428+
const char** argv,
4429+
int* exec_argc,
4430+
const char*** exec_argv,
4431+
bool is_env = false) {
4432+
// Parse a few arguments which are specific to Node.
4433+
int v8_argc;
4434+
const char** v8_argv;
4435+
ParseArgs(argc, argv, exec_argc, exec_argv, &v8_argc, &v8_argv, is_env);
4436+
4437+
// TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler
4438+
// manually? That would give us a little more control over its runtime
4439+
// behavior but it could also interfere with the user's intentions in ways
4440+
// we fail to anticipate. Dillema.
4441+
for (int i = 1; i < v8_argc; ++i) {
4442+
if (strncmp(v8_argv[i], "--prof", sizeof("--prof") - 1) == 0) {
4443+
v8_is_profiling = true;
4444+
break;
4445+
}
4446+
}
4447+
4448+
#ifdef __POSIX__
4449+
// Block SIGPROF signals when sleeping in epoll_wait/kevent/etc. Avoids the
4450+
// performance penalty of frequent EINTR wakeups when the profiler is running.
4451+
// Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
4452+
if (v8_is_profiling) {
4453+
uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
4454+
}
4455+
#endif
4456+
4457+
// The const_cast doesn't violate conceptual const-ness. V8 doesn't modify
4458+
// the argv array or the elements it points to.
4459+
if (v8_argc > 1)
4460+
V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);
4461+
4462+
// Anything that's still in v8_argv is not a V8 or a node option.
4463+
for (int i = 1; i < v8_argc; i++) {
4464+
fprintf(stderr, "%s: bad option: %s\n", argv[0], v8_argv[i]);
4465+
}
4466+
delete[] v8_argv;
4467+
v8_argv = nullptr;
4468+
4469+
if (v8_argc > 1) {
4470+
exit(9);
4471+
}
4472+
}
4473+
4474+
43704475
void Init(int* argc,
43714476
const char** argv,
43724477
int* exec_argc,
@@ -4397,31 +4502,36 @@ void Init(int* argc,
43974502
if (config_warning_file.empty())
43984503
SafeGetenv("NODE_REDIRECT_WARNINGS", &config_warning_file);
43994504

4400-
// Parse a few arguments which are specific to Node.
4401-
int v8_argc;
4402-
const char** v8_argv;
4403-
ParseArgs(argc, argv, exec_argc, exec_argv, &v8_argc, &v8_argv);
4404-
4405-
// TODO(bnoordhuis) Intercept --prof arguments and start the CPU profiler
4406-
// manually? That would give us a little more control over its runtime
4407-
// behavior but it could also interfere with the user's intentions in ways
4408-
// we fail to anticipate. Dillema.
4409-
for (int i = 1; i < v8_argc; ++i) {
4410-
if (strncmp(v8_argv[i], "--prof", sizeof("--prof") - 1) == 0) {
4411-
v8_is_profiling = true;
4412-
break;
4505+
#if !defined(NODE_WITHOUT_NODE_OPTIONS)
4506+
std::string node_options;
4507+
if (SafeGetenv("NODE_OPTIONS", &node_options)) {
4508+
// Smallest tokens are 2-chars (a not space and a space), plus 2 extra
4509+
// pointers, for the prepended executable name, and appended NULL pointer.
4510+
size_t max_len = 2 + (node_options.length() + 1) / 2;
4511+
const char** argv_from_env = new const char*[max_len];
4512+
int argc_from_env = 0;
4513+
// [0] is expected to be the program name, fill it in from the real argv.
4514+
argv_from_env[argc_from_env++] = argv[0];
4515+
4516+
char* cstr = strdup(node_options.c_str());
4517+
char* initptr = cstr;
4518+
char* token;
4519+
while ((token = strtok(initptr, " "))) { // NOLINT(runtime/threadsafe_fn)
4520+
initptr = nullptr;
4521+
argv_from_env[argc_from_env++] = token;
44134522
}
4414-
}
4415-
4416-
#ifdef __POSIX__
4417-
// Block SIGPROF signals when sleeping in epoll_wait/kevent/etc. Avoids the
4418-
// performance penalty of frequent EINTR wakeups when the profiler is running.
4419-
// Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
4420-
if (v8_is_profiling) {
4421-
uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
4523+
argv_from_env[argc_from_env] = nullptr;
4524+
int exec_argc_;
4525+
const char** exec_argv_ = nullptr;
4526+
ProcessArgv(&argc_from_env, argv_from_env, &exec_argc_, &exec_argv_, true);
4527+
delete[] exec_argv_;
4528+
delete[] argv_from_env;
4529+
free(cstr);
44224530
}
44234531
#endif
44244532

4533+
ProcessArgv(argc, argv, exec_argc, exec_argv);
4534+
44254535
#if defined(NODE_HAVE_I18N_SUPPORT)
44264536
// If the parameter isn't given, use the env variable.
44274537
if (icu_data_dir.empty())
@@ -4433,21 +4543,6 @@ void Init(int* argc,
44334543
"(check NODE_ICU_DATA or --icu-data-dir parameters)\n");
44344544
}
44354545
#endif
4436-
// The const_cast doesn't violate conceptual const-ness. V8 doesn't modify
4437-
// the argv array or the elements it points to.
4438-
if (v8_argc > 1)
4439-
V8::SetFlagsFromCommandLine(&v8_argc, const_cast<char**>(v8_argv), true);
4440-
4441-
// Anything that's still in v8_argv is not a V8 or a node option.
4442-
for (int i = 1; i < v8_argc; i++) {
4443-
fprintf(stderr, "%s: bad option: %s\n", argv[0], v8_argv[i]);
4444-
}
4445-
delete[] v8_argv;
4446-
v8_argv = nullptr;
4447-
4448-
if (v8_argc > 1) {
4449-
exit(9);
4450-
}
44514546

44524547
// Unconditionally force typed arrays to allocate outside the v8 heap. This
44534548
// is to prevent memory pointers from being moved around that are returned by
+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (process.config.variables.node_without_node_options)
4+
return common.skip('missing NODE_OPTIONS support');
5+
6+
// Test options specified by env variable.
7+
8+
const assert = require('assert');
9+
const exec = require('child_process').execFile;
10+
11+
disallow('--version');
12+
disallow('-v');
13+
disallow('--help');
14+
disallow('-h');
15+
disallow('--eval');
16+
disallow('-e');
17+
disallow('--print');
18+
disallow('-p');
19+
disallow('-pe');
20+
disallow('--check');
21+
disallow('-c');
22+
disallow('--interactive');
23+
disallow('-i');
24+
disallow('--v8-options');
25+
disallow('--');
26+
27+
function disallow(opt) {
28+
const options = {env: {NODE_OPTIONS: opt}};
29+
exec(process.execPath, options, common.mustCall(function(err) {
30+
const message = err.message.split(/\r?\n/)[1];
31+
const expect = process.execPath + ': ' + opt +
32+
' is not allowed in NODE_OPTIONS';
33+
34+
assert.strictEqual(err.code, 9);
35+
assert.strictEqual(message, expect);
36+
}));
37+
}
38+
39+
const printA = require.resolve('../fixtures/printA.js');
40+
41+
expect('-r ' + printA, 'A\nB\n');
42+
expect('--no-deprecation', 'B\n');
43+
expect('--no-warnings', 'B\n');
44+
expect('--trace-warnings', 'B\n');
45+
expect('--trace-deprecation', 'B\n');
46+
expect('--trace-sync-io', 'B\n');
47+
expect('--track-heap-objects', 'B\n');
48+
expect('--throw-deprecation', 'B\n');
49+
expect('--zero-fill-buffers', 'B\n');
50+
expect('--v8-pool-size=10', 'B\n');
51+
expect('--use-openssl-ca', 'B\n');
52+
expect('--use-bundled-ca', 'B\n');
53+
expect('--openssl-config=_ossl_cfg', 'B\n');
54+
expect('--icu-data-dir=_d', 'B\n');
55+
56+
// V8 options
57+
expect('--max_old_space_size=0', 'B\n');
58+
59+
function expect(opt, want) {
60+
const printB = require.resolve('../fixtures/printB.js');
61+
const argv = [printB];
62+
const opts = {
63+
env: {NODE_OPTIONS: opt},
64+
maxBuffer: 1000000000,
65+
};
66+
exec(process.execPath, argv, opts, common.mustCall(function(err, stdout) {
67+
assert.ifError(err);
68+
if (!RegExp(want).test(stdout)) {
69+
console.error('For %j, failed to find %j in: <\n%s\n>',
70+
opt, expect, stdout);
71+
assert(false, 'Expected ' + expect);
72+
}
73+
}));
74+
}

0 commit comments

Comments
 (0)