vm: fix crash when setting __proto__ on context's globalThis

nodejs · May 9, 2023 · 32efab2 · 32efab2
1 parent dccd25e
commit 32efab2
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 5 deletions.
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -529,11 +529,11 @@ void ContextifyContext::PropertySetterCallback(
 
   if (ctx->sandbox()->Set(context, property, value).IsNothing()) return;
 
-  Local<Value> desc;
-  if (is_declared_on_sandbox &&
-      ctx->sandbox()
-          ->GetOwnPropertyDescriptor(context, property)
-          .ToLocal(&desc)) {
+  if (is_declared_on_sandbox) {
+    Local<Value> desc;
+    bool success = ctx->sandbox()->GetOwnPropertyDescriptor(context, property).ToLocal(&desc);
+    if (!success || desc->IsUndefined()) return;
+
     Environment* env = Environment::GetCurrent(context);
     Local<Object> desc_obj = desc.As<Object>();
 

diff --git a/test/parallel/test-vm-set-proto-null-on-globalthis.js b/test/parallel/test-vm-set-proto-null-on-globalthis.js
@@ -0,0 +1,13 @@
+'use strict';
+require('../common');
+
+// Setting __proto__ on vm context's globalThis should not causes a crash
+// Regression test for https://github.com/nodejs/node/issues/47798
+
+const vm = require('vm');
+const context = vm.createContext();
+
+const contextGlobalThis = vm.runInContext('this', context);
+
+// Should not crash.
+contextGlobalThis.__proto__ = null; // eslint-disable-line no-proto