Skip to content

Commit e6a7300

Browse files
himself65himself65
authored
process: disallow some uses of Object.defineProperty() on process.env
Disallow the use of Object.defineProperty() to hide entries in process.env or make them immutable. PR-URL: #28006 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent df20947 commit e6a7300

6 files changed

Lines changed: 151 additions & 3 deletions

File tree

‎doc/api/errors.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,6 +1921,13 @@ valid.
19211921
The imported module string is an invalid URL, package name, or package subpath
19221922
specifier.
19231923

1924+
<a id="ERR_INVALID_OBJECT_DEFINE_PROPERTY"></a>
1925+
1926+
### `ERR_INVALID_OBJECT_DEFINE_PROPERTY`
1927+
1928+
An error occurred while setting an invalid attribute on the property of
1929+
an object.
1930+
19241931
<a id="ERR_INVALID_PACKAGE_CONFIG"></a>
19251932

19261933
### `ERR_INVALID_PACKAGE_CONFIG`

‎src/node_env_var.cc‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ using v8::Nothing;
2828
using v8::Object;
2929
using v8::ObjectTemplate;
3030
using v8::PropertyCallbackInfo;
31+
using v8::PropertyDescriptor;
3132
using v8::PropertyHandlerFlags;
3233
using v8::ReadOnly;
3334
using v8::String;
@@ -396,11 +397,57 @@ static void EnvEnumerator(const PropertyCallbackInfo<Array>& info) {
396397
env->env_vars()->Enumerate(env->isolate()));
397398
}
398399

400+
static void EnvDefiner(Local<Name> property,
401+
const PropertyDescriptor& desc,
402+
const PropertyCallbackInfo<Value>& info) {
403+
Environment* env = Environment::GetCurrent(info);
404+
if (desc.has_value()) {
405+
if (!desc.has_writable() ||
406+
!desc.has_enumerable() ||
407+
!desc.has_configurable()) {
408+
THROW_ERR_INVALID_OBJECT_DEFINE_PROPERTY(env,
409+
"'process.env' only accepts a "
410+
"configurable, writable,"
411+
" and enumerable "
412+
"data descriptor");
413+
} else if (!desc.configurable() ||
414+
!desc.enumerable() ||
415+
!desc.writable()) {
416+
THROW_ERR_INVALID_OBJECT_DEFINE_PROPERTY(env,
417+
"'process.env' only accepts a "
418+
"configurable, writable,"
419+
" and enumerable "
420+
"data descriptor");
421+
} else {
422+
return EnvSetter(property, desc.value(), info);
423+
}
424+
} else if (desc.has_get() || desc.has_set()) {
425+
// we don't accept a getter/setter in 'process.env'
426+
THROW_ERR_INVALID_OBJECT_DEFINE_PROPERTY(env,
427+
"'process.env' does not accept an"
428+
"accessor(getter/setter)"
429+
" descriptor");
430+
} else {
431+
THROW_ERR_INVALID_OBJECT_DEFINE_PROPERTY(env,
432+
"'process.env' only accepts a "
433+
"configurable, writable,"
434+
" and enumerable "
435+
"data descriptor");
436+
}
437+
}
438+
399439
MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context, Isolate* isolate) {
400440
EscapableHandleScope scope(isolate);
401441
Local<ObjectTemplate> env_proxy_template = ObjectTemplate::New(isolate);
402442
env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
403-
EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, Local<Value>(),
443+
EnvGetter,
444+
EnvSetter,
445+
EnvQuery,
446+
EnvDeleter,
447+
EnvEnumerator,
448+
EnvDefiner,
449+
nullptr,
450+
Local<Value>(),
404451
PropertyHandlerFlags::kHasNoSideEffect));
405452
return scope.EscapeMaybe(env_proxy_template->NewInstance(context));
406453
}
@@ -411,6 +458,7 @@ void RegisterEnvVarExternalReferences(ExternalReferenceRegistry* registry) {
411458
registry->Register(EnvQuery);
412459
registry->Register(EnvDeleter);
413460
registry->Register(EnvEnumerator);
461+
registry->Register(EnvDefiner);
414462
}
415463
} // namespace node
416464

‎src/node_errors.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ void OnFatalError(const char* location, const char* message);
6464
V(ERR_INVALID_ARG_VALUE, TypeError) \
6565
V(ERR_OSSL_EVP_INVALID_DIGEST, Error) \
6666
V(ERR_INVALID_ARG_TYPE, TypeError) \
67+
V(ERR_INVALID_OBJECT_DEFINE_PROPERTY, TypeError) \
6768
V(ERR_INVALID_MODULE, Error) \
6869
V(ERR_INVALID_THIS, TypeError) \
6970
V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \

‎test/parallel/test-process-env-delete.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
5+
process.env.foo = 'foo';
6+
assert.strictEqual(process.env.foo, 'foo');
7+
process.env.foo = undefined;
8+
assert.strictEqual(process.env.foo, 'undefined');
9+
10+
process.env.foo = 'foo';
11+
assert.strictEqual(process.env.foo, 'foo');
12+
delete process.env.foo;
13+
assert.strictEqual(process.env.foo, undefined);

‎test/parallel/test-process-env-ignore-getter-setter.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
5+
assert.throws(
6+
() => {
7+
Object.defineProperty(process.env, 'foo', {
8+
value: 'foo1'
9+
});
10+
},
11+
{
12+
code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
13+
name: 'TypeError',
14+
message: '\'process.env\' only accepts a ' +
15+
'configurable, writable,' +
16+
' and enumerable data descriptor'
17+
}
18+
);
19+
20+
assert.strictEqual(process.env.foo, undefined);
21+
process.env.foo = 'foo2';
22+
assert.strictEqual(process.env.foo, 'foo2');
23+
24+
assert.throws(
25+
() => {
26+
Object.defineProperty(process.env, 'goo', {
27+
get() {
28+
return 'goo';
29+
},
30+
set() {}
31+
});
32+
},
33+
{
34+
code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
35+
name: 'TypeError',
36+
message: '\'process.env\' does not accept an' +
37+
'accessor(getter/setter) descriptor'
38+
}
39+
);
40+
41+
const attributes = ['configurable', 'writable', 'enumerable'];
42+
43+
attributes.forEach((attribute) => {
44+
assert.throws(
45+
() => {
46+
Object.defineProperty(process.env, 'goo', {
47+
[attribute]: false
48+
});
49+
},
50+
{
51+
code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
52+
name: 'TypeError',
53+
message: '\'process.env\' only accepts a ' +
54+
'configurable, writable,' +
55+
' and enumerable data descriptor'
56+
}
57+
);
58+
});
59+
60+
assert.strictEqual(process.env.goo, undefined);
61+
Object.defineProperty(process.env, 'goo', {
62+
value: 'goo',
63+
configurable: true,
64+
writable: true,
65+
enumerable: true
66+
});
67+
assert.strictEqual(process.env.goo, 'goo');

‎test/parallel/test-worker-process-env.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,20 @@ if (!workerData && process.argv[2] !== 'child') {
3939
process.env.SET_IN_WORKER = 'set';
4040
assert.strictEqual(process.env.SET_IN_WORKER, 'set');
4141

42-
Object.defineProperty(process.env, 'DEFINED_IN_WORKER', { value: 42 });
43-
assert.strictEqual(process.env.DEFINED_IN_WORKER, '42');
42+
assert.throws(
43+
() => {
44+
Object.defineProperty(process.env, 'DEFINED_IN_WORKER', {
45+
value: 42
46+
});
47+
},
48+
{
49+
code: 'ERR_INVALID_OBJECT_DEFINE_PROPERTY',
50+
name: 'TypeError',
51+
message: '\'process.env\' only accepts a configurable, ' +
52+
'writable, and enumerable data descriptor'
53+
}
54+
);
55+
4456

4557
const { stderr } =
4658
child_process.spawnSync(process.execPath, [__filename, 'child']);

0 commit comments

Comments
 (0)