Skip to content

Commit 88aab45

Browse files
tniessenaddaleax
tniessen
authored and
addaleax
committed
os,vm: fix segfaults and CHECK failure
Fixes multiple possible segmentation faults in node_contextify.cc and an assertion failure in node_os.cc Fixes: #12369 Fixes: #12370 PR-URL: #12371 Reviewed-By: Anna Henningsen <[email protected]>
1 parent 9d52222 commit 88aab45

File tree

3 files changed

+157
-48
lines changed

3 files changed

+157
-48
lines changed

‎src/node_contextify.cc

+113-47
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,13 @@ using v8::FunctionCallbackInfo;
4444
using v8::FunctionTemplate;
4545
using v8::HandleScope;
4646
using v8::Integer;
47+
using v8::Just;
4748
using v8::Local;
4849
using v8::Maybe;
4950
using v8::MaybeLocal;
5051
using v8::Name;
5152
using v8::NamedPropertyHandlerConfiguration;
53+
using v8::Nothing;
5254
using v8::Object;
5355
using v8::ObjectTemplate;
5456
using v8::Persistent;
@@ -547,17 +549,20 @@ class ContextifyScript : public BaseObject {
547549
Local<String> code = args[0]->ToString(env->isolate());
548550

549551
Local<Value> options = args[1];
550-
Local<String> filename = GetFilenameArg(env, options);
551-
Local<Integer> lineOffset = GetLineOffsetArg(env, options);
552-
Local<Integer> columnOffset = GetColumnOffsetArg(env, options);
553-
bool display_errors = GetDisplayErrorsArg(env, options);
552+
MaybeLocal<String> filename = GetFilenameArg(env, options);
553+
MaybeLocal<Integer> lineOffset = GetLineOffsetArg(env, options);
554+
MaybeLocal<Integer> columnOffset = GetColumnOffsetArg(env, options);
555+
Maybe<bool> maybe_display_errors = GetDisplayErrorsArg(env, options);
554556
MaybeLocal<Uint8Array> cached_data_buf = GetCachedData(env, options);
555-
bool produce_cached_data = GetProduceCachedData(env, options);
557+
Maybe<bool> maybe_produce_cached_data = GetProduceCachedData(env, options);
556558
if (try_catch.HasCaught()) {
557559
try_catch.ReThrow();
558560
return;
559561
}
560562

563+
bool display_errors = maybe_display_errors.ToChecked();
564+
bool produce_cached_data = maybe_produce_cached_data.ToChecked();
565+
561566
ScriptCompiler::CachedData* cached_data = nullptr;
562567
Local<Uint8Array> ui8;
563568
if (cached_data_buf.ToLocal(&ui8)) {
@@ -567,7 +572,8 @@ class ContextifyScript : public BaseObject {
567572
ui8->ByteLength());
568573
}
569574

570-
ScriptOrigin origin(filename, lineOffset, columnOffset);
575+
ScriptOrigin origin(filename.ToLocalChecked(), lineOffset.ToLocalChecked(),
576+
columnOffset.ToLocalChecked());
571577
ScriptCompiler::Source source(code, origin, cached_data);
572578
ScriptCompiler::CompileOptions compile_options =
573579
ScriptCompiler::kNoCompileOptions;
@@ -625,14 +631,18 @@ class ContextifyScript : public BaseObject {
625631

626632
// Assemble arguments
627633
TryCatch try_catch(args.GetIsolate());
628-
uint64_t timeout = GetTimeoutArg(env, args[0]);
629-
bool display_errors = GetDisplayErrorsArg(env, args[0]);
630-
bool break_on_sigint = GetBreakOnSigintArg(env, args[0]);
634+
Maybe<int64_t> maybe_timeout = GetTimeoutArg(env, args[0]);
635+
Maybe<bool> maybe_display_errors = GetDisplayErrorsArg(env, args[0]);
636+
Maybe<bool> maybe_break_on_sigint = GetBreakOnSigintArg(env, args[0]);
631637
if (try_catch.HasCaught()) {
632638
try_catch.ReThrow();
633639
return;
634640
}
635641

642+
int64_t timeout = maybe_timeout.ToChecked();
643+
bool display_errors = maybe_display_errors.ToChecked();
644+
bool break_on_sigint = maybe_break_on_sigint.ToChecked();
645+
636646
// Do the eval within this context
637647
EvalMachine(env, timeout, display_errors, break_on_sigint, args,
638648
&try_catch);
@@ -655,13 +665,17 @@ class ContextifyScript : public BaseObject {
655665
Local<Object> sandbox = args[0].As<Object>();
656666
{
657667
TryCatch try_catch(env->isolate());
658-
timeout = GetTimeoutArg(env, args[1]);
659-
display_errors = GetDisplayErrorsArg(env, args[1]);
660-
break_on_sigint = GetBreakOnSigintArg(env, args[1]);
668+
Maybe<int64_t> maybe_timeout = GetTimeoutArg(env, args[1]);
669+
Maybe<bool> maybe_display_errors = GetDisplayErrorsArg(env, args[1]);
670+
Maybe<bool> maybe_break_on_sigint = GetBreakOnSigintArg(env, args[1]);
661671
if (try_catch.HasCaught()) {
662672
try_catch.ReThrow();
663673
return;
664674
}
675+
676+
timeout = maybe_timeout.ToChecked();
677+
display_errors = maybe_display_errors.ToChecked();
678+
break_on_sigint = maybe_break_on_sigint.ToChecked();
665679
}
666680

667681
// Get the context from the sandbox
@@ -731,60 +745,82 @@ class ContextifyScript : public BaseObject {
731745
True(env->isolate()));
732746
}
733747

734-
static bool GetBreakOnSigintArg(Environment* env, Local<Value> options) {
748+
static Maybe<bool> GetBreakOnSigintArg(Environment* env,
749+
Local<Value> options) {
735750
if (options->IsUndefined() || options->IsString()) {
736-
return false;
751+
return Just(false);
737752
}
738753
if (!options->IsObject()) {
739754
env->ThrowTypeError("options must be an object");
740-
return false;
755+
return Nothing<bool>();
741756
}
742757

743758
Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "breakOnSigint");
744-
Local<Value> value = options.As<Object>()->Get(key);
745-
return value->IsTrue();
759+
MaybeLocal<Value> maybe_value =
760+
options.As<Object>()->Get(env->context(), key);
761+
if (maybe_value.IsEmpty())
762+
return Nothing<bool>();
763+
764+
Local<Value> value = maybe_value.ToLocalChecked();
765+
return Just(value->IsTrue());
746766
}
747767

748-
static int64_t GetTimeoutArg(Environment* env, Local<Value> options) {
768+
static Maybe<int64_t> GetTimeoutArg(Environment* env, Local<Value> options) {
749769
if (options->IsUndefined() || options->IsString()) {
750-
return -1;
770+
return Just<int64_t>(-1);
751771
}
752772
if (!options->IsObject()) {
753773
env->ThrowTypeError("options must be an object");
754-
return -1;
774+
return Nothing<int64_t>();
755775
}
756776

757-
Local<Value> value = options.As<Object>()->Get(env->timeout_string());
777+
MaybeLocal<Value> maybe_value =
778+
options.As<Object>()->Get(env->context(), env->timeout_string());
779+
if (maybe_value.IsEmpty())
780+
return Nothing<int64_t>();
781+
782+
Local<Value> value = maybe_value.ToLocalChecked();
758783
if (value->IsUndefined()) {
759-
return -1;
784+
return Just<int64_t>(-1);
760785
}
761-
int64_t timeout = value->IntegerValue();
762786

763-
if (timeout <= 0) {
787+
Maybe<int64_t> timeout = value->IntegerValue(env->context());
788+
789+
if (timeout.IsJust() && timeout.ToChecked() <= 0) {
764790
env->ThrowRangeError("timeout must be a positive number");
765-
return -1;
791+
return Nothing<int64_t>();
766792
}
793+
767794
return timeout;
768795
}
769796

770797

771-
static bool GetDisplayErrorsArg(Environment* env, Local<Value> options) {
798+
static Maybe<bool> GetDisplayErrorsArg(Environment* env,
799+
Local<Value> options) {
772800
if (options->IsUndefined() || options->IsString()) {
773-
return true;
801+
return Just(true);
774802
}
775803
if (!options->IsObject()) {
776804
env->ThrowTypeError("options must be an object");
777-
return false;
805+
return Nothing<bool>();
778806
}
779807

780808
Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "displayErrors");
781-
Local<Value> value = options.As<Object>()->Get(key);
809+
MaybeLocal<Value> maybe_value =
810+
options.As<Object>()->Get(env->context(), key);
811+
if (maybe_value.IsEmpty())
812+
return Nothing<bool>();
782813

783-
return value->IsUndefined() ? true : value->BooleanValue();
814+
Local<Value> value = maybe_value.ToLocalChecked();
815+
if (value->IsUndefined())
816+
return Just(true);
817+
818+
return value->BooleanValue(env->context());
784819
}
785820

786821

787-
static Local<String> GetFilenameArg(Environment* env, Local<Value> options) {
822+
static MaybeLocal<String> GetFilenameArg(Environment* env,
823+
Local<Value> options) {
788824
Local<String> defaultFilename =
789825
FIXED_ONE_BYTE_STRING(env->isolate(), "evalmachine.<anonymous>");
790826

@@ -800,11 +836,15 @@ class ContextifyScript : public BaseObject {
800836
}
801837

802838
Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "filename");
803-
Local<Value> value = options.As<Object>()->Get(key);
839+
MaybeLocal<Value> maybe_value =
840+
options.As<Object>()->Get(env->context(), key);
841+
if (maybe_value.IsEmpty())
842+
return MaybeLocal<String>();
804843

844+
Local<Value> value = maybe_value.ToLocalChecked();
805845
if (value->IsUndefined())
806846
return defaultFilename;
807-
return value->ToString(env->isolate());
847+
return value->ToString(env->context());
808848
}
809849

810850

@@ -813,7 +853,13 @@ class ContextifyScript : public BaseObject {
813853
if (!options->IsObject()) {
814854
return MaybeLocal<Uint8Array>();
815855
}
816-
Local<Value> value = options.As<Object>()->Get(env->cached_data_string());
856+
857+
MaybeLocal<Value> maybe_value =
858+
options.As<Object>()->Get(env->context(), env->cached_data_string());
859+
if (maybe_value.IsEmpty())
860+
return MaybeLocal<Uint8Array>();
861+
862+
Local<Value> value = maybe_value.ToLocalChecked();
817863
if (value->IsUndefined()) {
818864
return MaybeLocal<Uint8Array>();
819865
}
@@ -827,44 +873,64 @@ class ContextifyScript : public BaseObject {
827873
}
828874

829875

830-
static bool GetProduceCachedData(Environment* env, Local<Value> options) {
876+
static Maybe<bool> GetProduceCachedData(Environment* env,
877+
Local<Value> options) {
831878
if (!options->IsObject()) {
832-
return false;
879+
return Just(false);
833880
}
834-
Local<Value> value =
835-
options.As<Object>()->Get(env->produce_cached_data_string());
836881

837-
return value->IsTrue();
882+
MaybeLocal<Value> maybe_value =
883+
options.As<Object>()->Get(env->context(),
884+
env->produce_cached_data_string());
885+
if (maybe_value.IsEmpty())
886+
return Nothing<bool>();
887+
888+
Local<Value> value = maybe_value.ToLocalChecked();
889+
return Just(value->IsTrue());
838890
}
839891

840892

841-
static Local<Integer> GetLineOffsetArg(Environment* env,
842-
Local<Value> options) {
893+
static MaybeLocal<Integer> GetLineOffsetArg(Environment* env,
894+
Local<Value> options) {
843895
Local<Integer> defaultLineOffset = Integer::New(env->isolate(), 0);
844896

845897
if (!options->IsObject()) {
846898
return defaultLineOffset;
847899
}
848900

849901
Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "lineOffset");
850-
Local<Value> value = options.As<Object>()->Get(key);
902+
MaybeLocal<Value> maybe_value =
903+
options.As<Object>()->Get(env->context(), key);
904+
if (maybe_value.IsEmpty())
905+
return MaybeLocal<Integer>();
851906

852-
return value->IsUndefined() ? defaultLineOffset : value->ToInteger();
907+
Local<Value> value = maybe_value.ToLocalChecked();
908+
if (value->IsUndefined())
909+
return defaultLineOffset;
910+
911+
return value->ToInteger(env->context());
853912
}
854913

855914

856-
static Local<Integer> GetColumnOffsetArg(Environment* env,
857-
Local<Value> options) {
915+
static MaybeLocal<Integer> GetColumnOffsetArg(Environment* env,
916+
Local<Value> options) {
858917
Local<Integer> defaultColumnOffset = Integer::New(env->isolate(), 0);
859918

860919
if (!options->IsObject()) {
861920
return defaultColumnOffset;
862921
}
863922

864923
Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "columnOffset");
865-
Local<Value> value = options.As<Object>()->Get(key);
924+
MaybeLocal<Value> maybe_value =
925+
options.As<Object>()->Get(env->context(), key);
926+
if (maybe_value.IsEmpty())
927+
return MaybeLocal<Integer>();
928+
929+
Local<Value> value = maybe_value.ToLocalChecked();
930+
if (value->IsUndefined())
931+
return defaultColumnOffset;
866932

867-
return value->IsUndefined() ? defaultColumnOffset : value->ToInteger();
933+
return value->ToInteger(env->context());
868934
}
869935

870936

‎src/node_os.cc

+7-1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ using v8::Function;
5757
using v8::FunctionCallbackInfo;
5858
using v8::Integer;
5959
using v8::Local;
60+
using v8::MaybeLocal;
6061
using v8::Null;
6162
using v8::Number;
6263
using v8::Object;
@@ -339,7 +340,12 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
339340

340341
if (args[0]->IsObject()) {
341342
Local<Object> options = args[0].As<Object>();
342-
Local<Value> encoding_opt = options->Get(env->encoding_string());
343+
MaybeLocal<Value> maybe_encoding = options->Get(env->context(),
344+
env->encoding_string());
345+
if (maybe_encoding.IsEmpty())
346+
return;
347+
348+
Local<Value> encoding_opt = maybe_encoding.ToLocalChecked();
343349
encoding = ParseEncoding(env->isolate(), encoding_opt, UTF8);
344350
} else {
345351
encoding = UTF8;

‎test/parallel/test-regress-GH-12371.js

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const execFile = require('child_process').execFile;
5+
6+
const scripts = [
7+
`os.userInfo({
8+
get encoding() {
9+
throw new Error('xyz');
10+
}
11+
})`
12+
];
13+
14+
['filename', 'cachedData', 'produceCachedData', 'lineOffset', 'columnOffset']
15+
.forEach((prop) => {
16+
scripts.push(`vm.createScript('', {
17+
get ${prop} () {
18+
throw new Error('xyz');
19+
}
20+
})`);
21+
});
22+
23+
['breakOnSigint', 'timeout', 'displayErrors']
24+
.forEach((prop) => {
25+
scripts.push(`vm.createScript('').runInThisContext({
26+
get ${prop} () {
27+
throw new Error('xyz');
28+
}
29+
})`);
30+
});
31+
32+
scripts.forEach((script) => {
33+
const node = process.execPath;
34+
execFile(node, [ '-e', script ], common.mustCall((err, stdout, stderr) => {
35+
assert(stderr.includes('Error: xyz'), 'createScript crashes');
36+
}));
37+
});

0 commit comments

Comments
 (0)