Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
lib: add option to disable __proto__
Adds `--disable-proto` CLI option which can be set to `delete` or
`throw`.

Fixes #31951

PR-URL: #32279
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
  • Loading branch information
devsnek authored and targos committed Apr 28, 2020
1 parent ea4302b commit b598321
Show file tree
Hide file tree
Showing 10 changed files with 174 additions and 29 deletions.
10 changes: 10 additions & 0 deletions doc/api/cli.md
Expand Up @@ -127,6 +127,15 @@ added: v12.0.0
Specify the file name of the CPU profile generated by `--cpu-prof`.

### `--disable-proto=mode`
<!--YAML
added: REPLACEME
-->

Disable the `Object.prototype.__proto__` property. If `mode` is `delete`, the
property will be removed entirely. If `mode` is `throw`, accesses to the
property will throw an exception with the code `ERR_PROTO_ACCESS`.

### `--disallow-code-generation-from-strings`
<!-- YAML
added: v9.8.0
Expand Down Expand Up @@ -1132,6 +1141,7 @@ node --require "./a.js" --require "./b.js"

Node.js options that are allowed are:
<!-- node-options-node start -->
* `--disable-proto`
* `--enable-fips`
* `--enable-source-maps`
* `--experimental-import-meta-resolve`
Expand Down
11 changes: 11 additions & 0 deletions doc/api/errors.md
Expand Up @@ -1664,6 +1664,14 @@ The `package.json` [exports][] field does not export the requested subpath.
Because exports are encapsulated, private internal modules that are not exported
cannot be imported through the package resolution, unless using an absolute URL.

<a id="ERR_PROTO_ACCESS"></a>
### `ERR_PROTO_ACCESS`

Accessing `Object.prototype.__proto__` has been forbidden using
[`--disable-proto=throw`][]. [`Object.getPrototypeOf`][] and
[`Object.setPrototypeOf`][] should be used to get and set the prototype of an
object.

<a id="ERR_REQUIRE_ESM"></a>
### `ERR_REQUIRE_ESM`

Expand Down Expand Up @@ -2474,10 +2482,13 @@ This `Error` is thrown when a read is attempted on a TTY `WriteStream`,
such as `process.stdout.on('data')`.

[`'uncaughtException'`]: process.html#process_event_uncaughtexception
[`--disable-proto=throw`]: cli.html#cli_disable_proto_mode
[`--force-fips`]: cli.html#cli_force_fips
[`Class: assert.AssertionError`]: assert.html#assert_class_assert_assertionerror
[`ERR_INVALID_ARG_TYPE`]: #ERR_INVALID_ARG_TYPE
[`EventEmitter`]: events.html#events_class_eventemitter
[`Object.getPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getPrototypeOf
[`Object.setPrototypeOf`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf
[`REPL`]: repl.html
[`Writable`]: stream.html#stream_class_stream_writable
[`child_process`]: child_process.html
Expand Down
8 changes: 8 additions & 0 deletions doc/node.1
Expand Up @@ -100,6 +100,14 @@ The default is
File name of the V8 CPU profile generated with
.Fl -cpu-prof
.
.It Fl -disable-proto Ns = Ns Ar mode
Disable the `Object.prototype.__proto__` property. If
.Ar mode
is `delete`, the property will be removed entirely. If
.Ar mode
is `throw`, accesses to the property will throw an exception with the code
`ERR_PROTO_ACCESS`.
.
.It Fl -disallow-code-generation-from-strings
Make built-in language features like `eval` and `new Function` that generate
code from strings throw an exception instead. This does not affect the Node.js
Expand Down
32 changes: 32 additions & 0 deletions src/api/environment.cc
Expand Up @@ -14,6 +14,7 @@ using v8::Context;
using v8::EscapableHandleScope;
using v8::FinalizationGroup;
using v8::Function;
using v8::FunctionCallbackInfo;
using v8::HandleScope;
using v8::Isolate;
using v8::Local;
Expand All @@ -23,6 +24,7 @@ using v8::Null;
using v8::Object;
using v8::ObjectTemplate;
using v8::Private;
using v8::PropertyDescriptor;
using v8::String;
using v8::Value;

Expand Down Expand Up @@ -403,6 +405,10 @@ Local<Context> NewContext(Isolate* isolate,
return context;
}

void ProtoThrower(const FunctionCallbackInfo<Value>& info) {
THROW_ERR_PROTO_ACCESS(info.GetIsolate());
}

// This runs at runtime, regardless of whether the context
// is created from a snapshot.
void InitializeContextRuntime(Local<Context> context) {
Expand Down Expand Up @@ -431,6 +437,32 @@ void InitializeContextRuntime(Local<Context> context) {
Local<Object> atomics = atomics_v.As<Object>();
atomics->Delete(context, wake_string).FromJust();
}

// Remove __proto__
// https://github.com/nodejs/node/issues/31951
Local<String> object_string = FIXED_ONE_BYTE_STRING(isolate, "Object");
Local<String> prototype_string = FIXED_ONE_BYTE_STRING(isolate, "prototype");
Local<Object> prototype = context->Global()
->Get(context, object_string)
.ToLocalChecked()
.As<Object>()
->Get(context, prototype_string)
.ToLocalChecked()
.As<Object>();
Local<String> proto_string = FIXED_ONE_BYTE_STRING(isolate, "__proto__");
if (per_process::cli_options->disable_proto == "delete") {
prototype->Delete(context, proto_string).ToChecked();
} else if (per_process::cli_options->disable_proto == "throw") {
Local<Value> thrower =
Function::New(context, ProtoThrower).ToLocalChecked();
PropertyDescriptor descriptor(thrower, thrower);
descriptor.set_enumerable(false);
descriptor.set_configurable(true);
prototype->DefineProperty(context, proto_string, descriptor).ToChecked();
} else if (per_process::cli_options->disable_proto != "") {
// Validated in ProcessGlobalArgs
FatalError("InitializeContextRuntime()", "invalid --disable-proto mode");
}
}

bool InitializeContextForSnapshot(Local<Context> context) {
Expand Down
7 changes: 7 additions & 0 deletions src/node.cc
Expand Up @@ -670,6 +670,13 @@ int ProcessGlobalArgs(std::vector<std::string>* args,
}
}

if (per_process::cli_options->disable_proto != "delete" &&
per_process::cli_options->disable_proto != "throw" &&
per_process::cli_options->disable_proto != "") {
errors->emplace_back("invalid mode passed to --disable-proto");
return 12;
}

auto env_opts = per_process::cli_options->per_isolate->per_env;
if (std::find(v8_args.begin(), v8_args.end(),
"--abort-on-uncaught-exception") != v8_args.end() ||
Expand Down
60 changes: 32 additions & 28 deletions src/node_errors.h
Expand Up @@ -31,33 +31,34 @@ void OnFatalError(const char* location, const char* message);
// `node::ERR_INVALID_ARG_TYPE(isolate, "message")` returning
// a `Local<Value>` containing the TypeError with proper code and message

#define ERRORS_WITH_CODE(V) \
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, Error) \
V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \
V(ERR_BUFFER_TOO_LARGE, Error) \
V(ERR_CONSTRUCT_CALL_REQUIRED, TypeError) \
V(ERR_CONSTRUCT_CALL_INVALID, TypeError) \
V(ERR_CRYPTO_UNKNOWN_CIPHER, Error) \
V(ERR_CRYPTO_UNKNOWN_DH_GROUP, Error) \
V(ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE, Error) \
V(ERR_INVALID_ARG_VALUE, TypeError) \
V(ERR_OSSL_EVP_INVALID_DIGEST, Error) \
V(ERR_INVALID_ARG_TYPE, TypeError) \
V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
V(ERR_MISSING_ARGS, TypeError) \
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \
V(ERR_MISSING_PASSPHRASE, TypeError) \
V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \
V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \
V(ERR_OUT_OF_RANGE, RangeError) \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \
V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \
V(ERR_STRING_TOO_LONG, Error) \
V(ERR_TLS_INVALID_PROTOCOL_METHOD, TypeError) \
V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, TypeError) \
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, Error) \
V(ERR_VM_MODULE_CACHED_DATA_REJECTED, Error) \
#define ERRORS_WITH_CODE(V) \
V(ERR_BUFFER_CONTEXT_NOT_AVAILABLE, Error) \
V(ERR_BUFFER_OUT_OF_BOUNDS, RangeError) \
V(ERR_BUFFER_TOO_LARGE, Error) \
V(ERR_CONSTRUCT_CALL_REQUIRED, TypeError) \
V(ERR_CONSTRUCT_CALL_INVALID, TypeError) \
V(ERR_CRYPTO_UNKNOWN_CIPHER, Error) \
V(ERR_CRYPTO_UNKNOWN_DH_GROUP, Error) \
V(ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE, Error) \
V(ERR_INVALID_ARG_VALUE, TypeError) \
V(ERR_OSSL_EVP_INVALID_DIGEST, Error) \
V(ERR_INVALID_ARG_TYPE, TypeError) \
V(ERR_INVALID_TRANSFER_OBJECT, TypeError) \
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
V(ERR_MISSING_ARGS, TypeError) \
V(ERR_MISSING_MESSAGE_PORT_IN_TRANSFER_LIST, TypeError) \
V(ERR_MISSING_PASSPHRASE, TypeError) \
V(ERR_MISSING_PLATFORM_FOR_WORKER, Error) \
V(ERR_NON_CONTEXT_AWARE_DISABLED, Error) \
V(ERR_OUT_OF_RANGE, RangeError) \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \
V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \
V(ERR_STRING_TOO_LONG, Error) \
V(ERR_TLS_INVALID_PROTOCOL_METHOD, TypeError) \
V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, TypeError) \
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, Error) \
V(ERR_VM_MODULE_CACHED_DATA_REJECTED, Error) \
V(ERR_PROTO_ACCESS, Error)

#define V(code, type) \
inline v8::Local<v8::Value> code(v8::Isolate* isolate, \
Expand Down Expand Up @@ -105,7 +106,10 @@ void OnFatalError(const char* location, const char* message);
"Script execution was interrupted by `SIGINT`") \
V(ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER, \
"Cannot serialize externalized SharedArrayBuffer") \
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, "Failed to set PSK identity hint")
V(ERR_TLS_PSK_SET_IDENTIY_HINT_FAILED, "Failed to set PSK identity hint") \
V(ERR_PROTO_ACCESS, \
"Accessing Object.prototype.__proto__ has been " \
"disallowed with --disable-proto=throw")

#define V(code, message) \
inline v8::Local<v8::Value> code(v8::Isolate* isolate) { \
Expand Down
5 changes: 4 additions & 1 deletion src/node_options.cc
Expand Up @@ -683,7 +683,10 @@ PerProcessOptionsParser::PerProcessOptionsParser(
"", /* undocumented, only for debugging */
&PerProcessOptions::debug_arraybuffer_allocations,
kAllowedInEnvironment);

AddOption("--disable-proto",
"disable Object.prototype.__proto__",
&PerProcessOptions::disable_proto,
kAllowedInEnvironment);

// 12.x renamed this inadvertently, so alias it for consistency within the
// release line, while using the original name for consistency with older
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Expand Up @@ -207,6 +207,7 @@ class PerProcessOptions : public Options {
int64_t v8_thread_pool_size = 4;
bool zero_fill_all_buffers = false;
bool debug_arraybuffer_allocations = false;
std::string disable_proto;

std::vector<std::string> security_reverts;
bool print_bash_completion = false;
Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-disable-proto-delete.js
@@ -0,0 +1,25 @@
// Flags: --disable-proto=delete

'use strict';

require('../common');
const assert = require('assert');
const vm = require('vm');
const { Worker, isMainThread } = require('worker_threads');

// eslint-disable-next-line no-proto
assert.strictEqual(Object.prototype.__proto__, undefined);
assert(!Object.prototype.hasOwnProperty('__proto__'));

const ctx = vm.createContext();
const ctxGlobal = vm.runInContext('this', ctx);

// eslint-disable-next-line no-proto
assert.strictEqual(ctxGlobal.Object.prototype.__proto__, undefined);
assert(!ctxGlobal.Object.prototype.hasOwnProperty('__proto__'));

if (isMainThread) {
new Worker(__filename);
} else {
process.exit();
}
44 changes: 44 additions & 0 deletions test/parallel/test-disable-proto-throw.js
@@ -0,0 +1,44 @@
// Flags: --disable-proto=throw

'use strict';

require('../common');
const assert = require('assert');
const vm = require('vm');
const { Worker, isMainThread } = require('worker_threads');

assert(Object.prototype.hasOwnProperty('__proto__'));

assert.throws(() => {
// eslint-disable-next-line no-proto
({}).__proto__;
}, {
code: 'ERR_PROTO_ACCESS'
});

assert.throws(() => {
// eslint-disable-next-line no-proto
({}).__proto__ = {};
}, {
code: 'ERR_PROTO_ACCESS',
});

const ctx = vm.createContext();

assert.throws(() => {
vm.runInContext('({}).__proto__;', ctx);
}, {
code: 'ERR_PROTO_ACCESS'
});

assert.throws(() => {
vm.runInContext('({}).__proto__ = {};', ctx);
}, {
code: 'ERR_PROTO_ACCESS',
});

if (isMainThread) {
new Worker(__filename);
} else {
process.exit();
}

0 comments on commit b598321

Please sign in to comment.