Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vm: add code generation options #19016

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions doc/api/vm.md
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,14 @@ added: v0.3.1
value of the [`url.origin`][] property of a [`URL`][] object. Most notably,
this string should omit the trailing slash, as that denotes a path.
**Default:** `''`.
* `contextCodeGeneration` {Object}
* `strings` {boolean} If set to false any calls to `eval` or function
constructors (`Function`, `GeneratorFunction`, etc) will throw an
`EvalError`.
**Default**: `true`.
* `wasm` {boolean} If set to false any attempt to compile a WebAssembly
module will throw a `WebAssembly.CompileError`.
**Default**: `true`.

First contextifies the given `sandbox`, runs the compiled code contained by
the `vm.Script` object within the created sandbox, and returns the result.
Expand Down Expand Up @@ -578,6 +586,14 @@ added: v0.3.1
the [`url.origin`][] property of a [`URL`][] object. Most notably, this
string should omit the trailing slash, as that denotes a path.
**Default:** `''`.
* `codeGeneration` {Object}
* `strings` {boolean} If set to false any calls to `eval` or function
constructors (`Function`, `GeneratorFunction`, etc) will throw an
`EvalError`.
**Default**: `true`.
* `wasm` {boolean} If set to false any attempt to compile a WebAssembly
module will throw a `WebAssembly.CompileError`.
**Default**: `true`.

If given a `sandbox` object, the `vm.createContext()` method will [prepare
that sandbox][contextified] so that it can be used in calls to
Expand Down
37 changes: 35 additions & 2 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,36 @@ function validateString(prop, propName) {
throw new ERR_INVALID_ARG_TYPE(propName, 'string', prop);
}

function validateBool(prop, propName) {
if (prop !== undefined && typeof prop !== 'boolean')
throw new ERR_INVALID_ARG_TYPE(propName, 'boolean', prop);
}

function validateObject(prop, propName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rather name this function validateObjectOrUndefined?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none of the others are named like that, and it seems fairly self-explanatory

if (prop !== undefined && (typeof prop !== 'object' || prop === null))
throw new ERR_INVALID_ARG_TYPE(propName, 'Object', prop);
}

function getContextOptions(options) {
if (options) {
validateObject(options.contextCodeGeneration,
'options.contextCodeGeneration');
const contextOptions = {
name: options.contextName,
origin: options.contextOrigin
origin: options.contextOrigin,
codeGeneration: typeof options.contextCodeGeneration === 'object' ? {
strings: options.contextCodeGeneration.strings,
wasm: options.contextCodeGeneration.wasm,
} : undefined,
};
validateString(contextOptions.name, 'options.contextName');
validateString(contextOptions.origin, 'options.contextOrigin');
if (contextOptions.codeGeneration) {
validateBool(contextOptions.codeGeneration.strings,
'options.contextCodeGeneration.strings');
validateBool(contextOptions.codeGeneration.wasm,
'options.contextCodeGeneration.wasm');
}
return contextOptions;
}
return {};
Expand All @@ -109,10 +131,21 @@ function createContext(sandbox, options) {
if (typeof options !== 'object' || options === null) {
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
}
validateObject(options.codeGeneration, 'options.codeGeneration');
options = {
name: options.name,
origin: options.origin
origin: options.origin,
codeGeneration: typeof options.codeGeneration === 'object' ? {
strings: options.codeGeneration.strings,
wasm: options.codeGeneration.wasm,
} : undefined,
};
if (options.codeGeneration !== undefined) {
validateBool(options.codeGeneration.strings,
'options.codeGeneration.strings');
validateBool(options.codeGeneration.wasm,
'options.codeGeneration.wasm');
}
if (options.name === undefined) {
options.name = `VM Context ${defaultContextNameIndex++}`;
} else if (typeof options.name !== 'string') {
Expand Down
11 changes: 11 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "node_revert.h"
#include "node_debug_options.h"
#include "node_perf.h"
#include "node_context_data.h"

#if defined HAVE_PERFCTR
#include "node_counters.h"
Expand Down Expand Up @@ -4421,6 +4422,8 @@ Local<Context> NewContext(Isolate* isolate,
HandleScope handle_scope(isolate);
auto intl_key = FIXED_ONE_BYTE_STRING(isolate, "Intl");
auto break_iter_key = FIXED_ONE_BYTE_STRING(isolate, "v8BreakIterator");
context->SetEmbedderData(
ContextEmbedderIndex::kAllowWasmCodeGeneration, True(isolate));
Local<Value> intl_v;
if (context->Global()->Get(context, intl_key).ToLocal(&intl_v) &&
intl_v->IsObject()) {
Expand Down Expand Up @@ -4498,6 +4501,13 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
return exit_code;
}

bool AllowWasmCodeGenerationCallback(
Local<Context> context, Local<String>) {
Local<Value> wasm_code_gen =
context->GetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration);
return wasm_code_gen->IsUndefined() || wasm_code_gen->IsTrue();
}

inline int Start(uv_loop_t* event_loop,
int argc, const char* const* argv,
int exec_argc, const char* const* exec_argv) {
Expand All @@ -4516,6 +4526,7 @@ inline int Start(uv_loop_t* event_loop,
isolate->SetAbortOnUncaughtExceptionCallback(ShouldAbortOnUncaughtException);
isolate->SetMicrotasksPolicy(v8::MicrotasksPolicy::kExplicit);
isolate->SetFatalErrorHandler(OnFatalError);
isolate->SetAllowWasmCodeGenerationCallback(AllowWasmCodeGenerationCallback);

{
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
Expand Down
5 changes: 5 additions & 0 deletions src/node_context_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ namespace node {
#define NODE_CONTEXT_SANDBOX_OBJECT_INDEX 33
#endif

#ifndef NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX
#define NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX 34
#endif

enum ContextEmbedderIndex {
kEnvironment = NODE_CONTEXT_EMBEDDER_DATA_INDEX,
kSandboxObject = NODE_CONTEXT_SANDBOX_OBJECT_INDEX,
kAllowWasmCodeGeneration = NODE_CONTEXT_ALLOW_WASM_CODE_GENERATION_INDEX,
};

} // namespace node
Expand Down
24 changes: 24 additions & 0 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,30 @@ Local<Context> ContextifyContext::CreateV8Context(
CHECK(name->IsString());
Utf8Value name_val(env->isolate(), name);

Local<Value> codegen = options_obj->Get(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "codeGeneration"))
.ToLocalChecked();

if (!codegen->IsUndefined()) {
CHECK(codegen->IsObject());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could leave a comment here, that the js part already ensured that we have undefined or an object. Looking only at the C++ part, my first response was that this CHECK would fail a lot.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pattern it used a lot with stuff that is validated js-side and we don't leave usually comments, are you worried about something specific not being construed?

Local<Object> codegen_obj = codegen.As<Object>();

Local<Value> allow_code_gen_from_strings =
codegen_obj->Get(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "strings"))
.ToLocalChecked();
ctx->AllowCodeGenerationFromStrings(
allow_code_gen_from_strings->IsUndefined() ||
allow_code_gen_from_strings->IsTrue());

Local<Value> allow_wasm_code_gen = codegen_obj->Get(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "wasm"))
.ToLocalChecked();
ctx->SetEmbedderData(ContextEmbedderIndex::kAllowWasmCodeGeneration,
Boolean::New(env->isolate(), allow_wasm_code_gen->IsUndefined() ||
allow_wasm_code_gen->IsTrue()));
}

ContextInfo info(*name_val);

Local<Value> origin =
Expand Down
3 changes: 3 additions & 0 deletions src/node_contextify.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class ContextifyContext {
v8::Local<v8::Object> sandbox_obj, v8::Local<v8::Object> options_obj);
static void Init(Environment* env, v8::Local<v8::Object> target);

static bool AllowWasmCodeGeneration(
v8::Local<v8::Context> context, v8::Local<v8::String>);

static ContextifyContext* ContextFromContextifiedSandbox(
Environment* env,
const v8::Local<v8::Object>& sandbox);
Expand Down
110 changes: 110 additions & 0 deletions test/parallel/test-vm-codegen.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
'use strict';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs tests for unexpected types of contextCodeGeneration and its properties.


const common = require('../common');
const assert = require('assert');

const { createContext, runInContext, runInNewContext } = require('vm');

const WASM_BYTES = Buffer.from(
[0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00]);


function expectsError(fn, type) {
try {
fn();
assert.fail('expected fn to error');
} catch (err) {
if (typeof type === 'string')
assert.strictEqual(err.name, type);
else
assert(err instanceof type);
}
}

{
const ctx = createContext({ WASM_BYTES });
const test = 'eval(""); new WebAssembly.Module(WASM_BYTES);';
runInContext(test, ctx);

runInNewContext(test, { WASM_BYTES }, {
contextCodeGeneration: undefined,
});
}

{
const ctx = createContext({}, {
codeGeneration: {
strings: false,
},
});

const EvalError = runInContext('EvalError', ctx);
expectsError(() => {
runInContext('eval("x")', ctx);
}, EvalError);
}

{
const ctx = createContext({ WASM_BYTES }, {
codeGeneration: {
wasm: false,
},
});

const CompileError = runInContext('WebAssembly.CompileError', ctx);
expectsError(() => {
runInContext('new WebAssembly.Module(WASM_BYTES)', ctx);
}, CompileError);
}

expectsError(() => {
runInNewContext('eval("x")', {}, {
contextCodeGeneration: {
strings: false,
},
});
}, 'EvalError');

expectsError(() => {
runInNewContext('new WebAssembly.Module(WASM_BYTES)', { WASM_BYTES }, {
contextCodeGeneration: {
wasm: false,
},
});
}, 'CompileError');

common.expectsError(() => {
createContext({}, {
codeGeneration: {
strings: 0,
},
});
}, {
code: 'ERR_INVALID_ARG_TYPE',
});

common.expectsError(() => {
runInNewContext('eval("x")', {}, {
contextCodeGeneration: {
wasm: 1,
},
});
}, {
code: 'ERR_INVALID_ARG_TYPE'
});

common.expectsError(() => {
createContext({}, {
codeGeneration: 1,
});
}, {
code: 'ERR_INVALID_ARG_TYPE',
});

common.expectsError(() => {
createContext({}, {
codeGeneration: null,
});
}, {
code: 'ERR_INVALID_ARG_TYPE',
});