From 184fc2dd827940b4ef72d77bf77b837192efe37b Mon Sep 17 00:00:00 2001 From: Feng Yu Date: Tue, 9 May 2023 12:12:46 -0700 Subject: [PATCH 1/3] vm: fix crash when setting __proto__ on context's globalThis --- src/node_contextify.cc | 12 +++++++----- .../test-vm-set-proto-null-on-globalthis.js | 13 +++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-vm-set-proto-null-on-globalthis.js diff --git a/src/node_contextify.cc b/src/node_contextify.cc index a21acf06a32781..09cecf8bd8d19b 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -529,11 +529,13 @@ void ContextifyContext::PropertySetterCallback( if (ctx->sandbox()->Set(context, property, value).IsNothing()) return; - Local desc; - if (is_declared_on_sandbox && - ctx->sandbox() - ->GetOwnPropertyDescriptor(context, property) - .ToLocal(&desc)) { + if (is_declared_on_sandbox) { + Local desc; + bool success = ctx->sandbox() + ->GetOwnPropertyDescriptor(context, property) + .ToLocal(&desc); + if (!success || desc->IsUndefined()) return; + Environment* env = Environment::GetCurrent(context); Local desc_obj = desc.As(); diff --git a/test/parallel/test-vm-set-proto-null-on-globalthis.js b/test/parallel/test-vm-set-proto-null-on-globalthis.js new file mode 100644 index 00000000000000..38668ea83294ff --- /dev/null +++ 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 From 7e11275965474344952ff836b08890366fd0fe76 Mon Sep 17 00:00:00 2001 From: Feng Yu Date: Tue, 9 May 2023 18:09:43 -0700 Subject: [PATCH 2/3] fixup! vm: fix crash when setting __proto__ on context's globalThis --- src/node_contextify.cc | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 09cecf8bd8d19b..f1933ca5ed4c2c 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -529,13 +529,12 @@ void ContextifyContext::PropertySetterCallback( if (ctx->sandbox()->Set(context, property, value).IsNothing()) return; - if (is_declared_on_sandbox) { - Local desc; - bool success = ctx->sandbox() - ->GetOwnPropertyDescriptor(context, property) - .ToLocal(&desc); - if (!success || desc->IsUndefined()) return; - + Local desc; + if (is_declared_on_sandbox && + ctx->sandbox() + ->GetOwnPropertyDescriptor(context, property) + .ToLocal(&desc) && + !desc->IsUndefined()) { Environment* env = Environment::GetCurrent(context); Local desc_obj = desc.As(); From 4bfdfc547f737bff1a394b7d2f8b41eaa6747ff1 Mon Sep 17 00:00:00 2001 From: Feng Yu Date: Wed, 10 May 2023 08:31:23 -0700 Subject: [PATCH 3/3] Update test/parallel/test-vm-set-proto-null-on-globalthis.js Co-authored-by: Mohammed Keyvanzadeh --- test/parallel/test-vm-set-proto-null-on-globalthis.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-vm-set-proto-null-on-globalthis.js b/test/parallel/test-vm-set-proto-null-on-globalthis.js index 38668ea83294ff..869124fa86d1f5 100644 --- a/test/parallel/test-vm-set-proto-null-on-globalthis.js +++ b/test/parallel/test-vm-set-proto-null-on-globalthis.js @@ -1,7 +1,7 @@ 'use strict'; require('../common'); -// Setting __proto__ on vm context's globalThis should not causes a crash +// Setting __proto__ on vm context's globalThis should not cause a crash // Regression test for https://github.com/nodejs/node/issues/47798 const vm = require('vm');