From 47e702059d2360f5e2dec9c05e829222b1c8f9aa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Wed, 12 Apr 2017 20:57:28 +0200
Subject: [PATCH] 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: https://github.com/nodejs/node/issues/12369
Fixes: https://github.com/nodejs/node/issues/12370
Backport-PR-URL: https://github.com/nodejs/node/pull/13871
PR-URL: https://github.com/nodejs/node/pull/12371
Reviewed-By: Anna Henningsen <anna@addaleax.net>
---
 src/node_contextify.cc                 | 161 +++++++++++++++++--------
 src/node_os.cc                         |   8 +-
 test/parallel/test-regress-GH-12371.js |  37 ++++++
 3 files changed, 157 insertions(+), 49 deletions(-)
 create mode 100644 test/parallel/test-regress-GH-12371.js

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 741f568259a1a6..1ea8fa0b1cb6bf 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -23,11 +23,13 @@ using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Integer;
+using v8::Just;
 using v8::Local;
 using v8::Maybe;
 using v8::MaybeLocal;
 using v8::Name;
 using v8::NamedPropertyHandlerConfiguration;
+using v8::Nothing;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::Persistent;
@@ -495,17 +497,20 @@ class ContextifyScript : public BaseObject {
     Local<String> code = args[0]->ToString(env->isolate());
 
     Local<Value> options = args[1];
-    Local<String> filename = GetFilenameArg(env, options);
-    Local<Integer> lineOffset = GetLineOffsetArg(env, options);
-    Local<Integer> columnOffset = GetColumnOffsetArg(env, options);
-    bool display_errors = GetDisplayErrorsArg(env, options);
+    MaybeLocal<String> filename = GetFilenameArg(env, options);
+    MaybeLocal<Integer> lineOffset = GetLineOffsetArg(env, options);
+    MaybeLocal<Integer> columnOffset = GetColumnOffsetArg(env, options);
+    Maybe<bool> maybe_display_errors = GetDisplayErrorsArg(env, options);
     MaybeLocal<Uint8Array> cached_data_buf = GetCachedData(env, options);
-    bool produce_cached_data = GetProduceCachedData(env, options);
+    Maybe<bool> maybe_produce_cached_data = GetProduceCachedData(env, options);
     if (try_catch.HasCaught()) {
       try_catch.ReThrow();
       return;
     }
 
+    bool display_errors = maybe_display_errors.FromJust();
+    bool produce_cached_data = maybe_produce_cached_data.FromJust();
+
     ScriptCompiler::CachedData* cached_data = nullptr;
     if (!cached_data_buf.IsEmpty()) {
       Local<Uint8Array> ui8 = cached_data_buf.ToLocalChecked();
@@ -515,7 +520,8 @@ class ContextifyScript : public BaseObject {
           ui8->ByteLength());
     }
 
-    ScriptOrigin origin(filename, lineOffset, columnOffset);
+    ScriptOrigin origin(filename.ToLocalChecked(), lineOffset.ToLocalChecked(),
+                        columnOffset.ToLocalChecked());
     ScriptCompiler::Source source(code, origin, cached_data);
     ScriptCompiler::CompileOptions compile_options =
         ScriptCompiler::kNoCompileOptions;
@@ -573,14 +579,18 @@ class ContextifyScript : public BaseObject {
 
     // Assemble arguments
     TryCatch try_catch(args.GetIsolate());
-    uint64_t timeout = GetTimeoutArg(env, args[0]);
-    bool display_errors = GetDisplayErrorsArg(env, args[0]);
-    bool break_on_sigint = GetBreakOnSigintArg(env, args[0]);
+    Maybe<int64_t> maybe_timeout = GetTimeoutArg(env, args[0]);
+    Maybe<bool> maybe_display_errors = GetDisplayErrorsArg(env, args[0]);
+    Maybe<bool> maybe_break_on_sigint = GetBreakOnSigintArg(env, args[0]);
     if (try_catch.HasCaught()) {
       try_catch.ReThrow();
       return;
     }
 
+    int64_t timeout = maybe_timeout.FromJust();
+    bool display_errors = maybe_display_errors.FromJust();
+    bool break_on_sigint = maybe_break_on_sigint.FromJust();
+
     // Do the eval within this context
     EvalMachine(env, timeout, display_errors, break_on_sigint, args,
                 &try_catch);
@@ -603,13 +613,17 @@ class ContextifyScript : public BaseObject {
     Local<Object> sandbox = args[0].As<Object>();
     {
       TryCatch try_catch(env->isolate());
-      timeout = GetTimeoutArg(env, args[1]);
-      display_errors = GetDisplayErrorsArg(env, args[1]);
-      break_on_sigint = GetBreakOnSigintArg(env, args[1]);
+      Maybe<int64_t> maybe_timeout = GetTimeoutArg(env, args[1]);
+      Maybe<bool> maybe_display_errors = GetDisplayErrorsArg(env, args[1]);
+      Maybe<bool> maybe_break_on_sigint = GetBreakOnSigintArg(env, args[1]);
       if (try_catch.HasCaught()) {
         try_catch.ReThrow();
         return;
       }
+
+      timeout = maybe_timeout.FromJust();
+      display_errors = maybe_display_errors.FromJust();
+      break_on_sigint = maybe_break_on_sigint.FromJust();
     }
 
     // Get the context from the sandbox
@@ -679,61 +693,82 @@ class ContextifyScript : public BaseObject {
         True(env->isolate()));
   }
 
-  static bool GetBreakOnSigintArg(Environment* env, Local<Value> options) {
+  static Maybe<bool> GetBreakOnSigintArg(Environment* env,
+                                         Local<Value> options) {
     if (options->IsUndefined() || options->IsString()) {
-      return false;
+      return Just(false);
     }
     if (!options->IsObject()) {
       env->ThrowTypeError("options must be an object");
-      return false;
+      return Nothing<bool>();
     }
 
     Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "breakOnSigint");
-    Local<Value> value = options.As<Object>()->Get(key);
-    return value->IsTrue();
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(), key);
+    if (maybe_value.IsEmpty())
+      return Nothing<bool>();
+
+    Local<Value> value = maybe_value.ToLocalChecked();
+    return Just(value->IsTrue());
   }
 
-  static int64_t GetTimeoutArg(Environment* env, Local<Value> options) {
+  static Maybe<int64_t> GetTimeoutArg(Environment* env, Local<Value> options) {
     if (options->IsUndefined() || options->IsString()) {
-      return -1;
+      return Just<int64_t>(-1);
     }
     if (!options->IsObject()) {
       env->ThrowTypeError("options must be an object");
-      return -1;
+      return Nothing<int64_t>();
     }
 
-    Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "timeout");
-    Local<Value> value = options.As<Object>()->Get(key);
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(), env->timeout_string());
+    if (maybe_value.IsEmpty())
+      return Nothing<int64_t>();
+
+    Local<Value> value = maybe_value.ToLocalChecked();
     if (value->IsUndefined()) {
-      return -1;
+      return Just<int64_t>(-1);
     }
-    int64_t timeout = value->IntegerValue();
 
-    if (timeout <= 0) {
+    Maybe<int64_t> timeout = value->IntegerValue(env->context());
+
+    if (timeout.IsJust() && timeout.FromJust() <= 0) {
       env->ThrowRangeError("timeout must be a positive number");
-      return -1;
+      return Nothing<int64_t>();
     }
+
     return timeout;
   }
 
 
-  static bool GetDisplayErrorsArg(Environment* env, Local<Value> options) {
+  static Maybe<bool> GetDisplayErrorsArg(Environment* env,
+                                         Local<Value> options) {
     if (options->IsUndefined() || options->IsString()) {
-      return true;
+      return Just(true);
     }
     if (!options->IsObject()) {
       env->ThrowTypeError("options must be an object");
-      return false;
+      return Nothing<bool>();
     }
 
     Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "displayErrors");
-    Local<Value> value = options.As<Object>()->Get(key);
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(), key);
+    if (maybe_value.IsEmpty())
+      return Nothing<bool>();
 
-    return value->IsUndefined() ? true : value->BooleanValue();
+    Local<Value> value = maybe_value.ToLocalChecked();
+    if (value->IsUndefined())
+      return Just(true);
+
+    return value->BooleanValue(env->context());
   }
 
 
-  static Local<String> GetFilenameArg(Environment* env, Local<Value> options) {
+  static MaybeLocal<String> GetFilenameArg(Environment* env,
+                                           Local<Value> options) {
     Local<String> defaultFilename =
         FIXED_ONE_BYTE_STRING(env->isolate(), "evalmachine.<anonymous>");
 
@@ -749,11 +784,15 @@ class ContextifyScript : public BaseObject {
     }
 
     Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "filename");
-    Local<Value> value = options.As<Object>()->Get(key);
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(), key);
+    if (maybe_value.IsEmpty())
+      return MaybeLocal<String>();
 
+    Local<Value> value = maybe_value.ToLocalChecked();
     if (value->IsUndefined())
       return defaultFilename;
-    return value->ToString(env->isolate());
+    return value->ToString(env->context());
   }
 
 
@@ -762,7 +801,13 @@ class ContextifyScript : public BaseObject {
     if (!options->IsObject()) {
       return MaybeLocal<Uint8Array>();
     }
-    Local<Value> value = options.As<Object>()->Get(env->cached_data_string());
+
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(), env->cached_data_string());
+    if (maybe_value.IsEmpty())
+      return MaybeLocal<Uint8Array>();
+
+    Local<Value> value = maybe_value.ToLocalChecked();
     if (value->IsUndefined()) {
       return MaybeLocal<Uint8Array>();
     }
@@ -776,19 +821,25 @@ class ContextifyScript : public BaseObject {
   }
 
 
-  static bool GetProduceCachedData(Environment* env, Local<Value> options) {
+  static Maybe<bool> GetProduceCachedData(Environment* env,
+                                          Local<Value> options) {
     if (!options->IsObject()) {
-      return false;
+      return Just(false);
     }
-    Local<Value> value =
-        options.As<Object>()->Get(env->produce_cached_data_string());
 
-    return value->IsTrue();
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(),
+                                  env->produce_cached_data_string());
+    if (maybe_value.IsEmpty())
+      return Nothing<bool>();
+
+    Local<Value> value = maybe_value.ToLocalChecked();
+    return Just(value->IsTrue());
   }
 
 
-  static Local<Integer> GetLineOffsetArg(Environment* env,
-                                         Local<Value> options) {
+  static MaybeLocal<Integer> GetLineOffsetArg(Environment* env,
+                                              Local<Value> options) {
     Local<Integer> defaultLineOffset = Integer::New(env->isolate(), 0);
 
     if (!options->IsObject()) {
@@ -796,14 +847,21 @@ class ContextifyScript : public BaseObject {
     }
 
     Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "lineOffset");
-    Local<Value> value = options.As<Object>()->Get(key);
+    MaybeLocal<Value> maybe_value =
+        options.As<Object>()->Get(env->context(), key);
+    if (maybe_value.IsEmpty())
+      return MaybeLocal<Integer>();
 
-    return value->IsUndefined() ? defaultLineOffset : value->ToInteger();
+    Local<Value> value = maybe_value.ToLocalChecked();
+    if (value->IsUndefined())
+      return defaultLineOffset;
+
+    return value->ToInteger(env->context());
   }
 
 
-  static Local<Integer> GetColumnOffsetArg(Environment* env,
-                                           Local<Value> options) {
+  static MaybeLocal<Integer> GetColumnOffsetArg(Environment* env,
+                                                Local<Value> options) {
     Local<Integer> defaultColumnOffset = Integer::New(env->isolate(), 0);
 
     if (!options->IsObject()) {
@@ -811,9 +869,16 @@ class ContextifyScript : public BaseObject {
     }
 
     Local<String> key = FIXED_ONE_BYTE_STRING(env->isolate(), "columnOffset");
-    Local<Value> value = options.As<Object>()->Get(key);
+    MaybeLocal<Value> maybe_value =
+      options.As<Object>()->Get(env->context(), key);
+    if (maybe_value.IsEmpty())
+      return MaybeLocal<Integer>();
+
+    Local<Value> value = maybe_value.ToLocalChecked();
+    if (value->IsUndefined())
+      return defaultColumnOffset;
 
-    return value->IsUndefined() ? defaultColumnOffset : value->ToInteger();
+    return value->ToInteger(env->context());
   }
 
 
diff --git a/src/node_os.cc b/src/node_os.cc
index 211ac3d01dd8b2..19eaaeeb301d88 100644
--- a/src/node_os.cc
+++ b/src/node_os.cc
@@ -35,6 +35,7 @@ using v8::Float64Array;
 using v8::FunctionCallbackInfo;
 using v8::Integer;
 using v8::Local;
+using v8::MaybeLocal;
 using v8::Null;
 using v8::Number;
 using v8::Object;
@@ -306,7 +307,12 @@ static void GetUserInfo(const FunctionCallbackInfo<Value>& args) {
 
   if (args[0]->IsObject()) {
     Local<Object> options = args[0].As<Object>();
-    Local<Value> encoding_opt = options->Get(env->encoding_string());
+    MaybeLocal<Value> maybe_encoding = options->Get(env->context(),
+                                                    env->encoding_string());
+    if (maybe_encoding.IsEmpty())
+      return;
+
+    Local<Value> encoding_opt = maybe_encoding.ToLocalChecked();
     encoding = ParseEncoding(env->isolate(), encoding_opt, UTF8);
   } else {
     encoding = UTF8;
diff --git a/test/parallel/test-regress-GH-12371.js b/test/parallel/test-regress-GH-12371.js
new file mode 100644
index 00000000000000..6ab65a8e348e1e
--- /dev/null
+++ b/test/parallel/test-regress-GH-12371.js
@@ -0,0 +1,37 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const execFile = require('child_process').execFile;
+
+const scripts = [
+  `os.userInfo({
+    get encoding() {
+      throw new Error('xyz');
+    }
+  })`
+];
+
+['filename', 'cachedData', 'produceCachedData', 'lineOffset', 'columnOffset']
+  .forEach((prop) => {
+    scripts.push(`vm.createScript('', {
+      get ${prop} () {
+        throw new Error('xyz');
+      }
+    })`);
+  });
+
+['breakOnSigint', 'timeout', 'displayErrors']
+  .forEach((prop) => {
+    scripts.push(`vm.createScript('').runInThisContext({
+      get ${prop} () {
+        throw new Error('xyz');
+      }
+    })`);
+  });
+
+scripts.forEach((script) => {
+  const node = process.execPath;
+  execFile(node, [ '-e', script ], common.mustCall((err, stdout, stderr) => {
+    assert(stderr.includes('Error: xyz'), 'createScript crashes');
+  }));
+});