From d69a04dd77f6cc13ea6aa8b58f586f32e3c80a90 Mon Sep 17 00:00:00 2001 From: Franziska Hinkelmann Date: Mon, 29 Jan 2024 19:12:20 -0500 Subject: [PATCH] src: fix vm bug for configurable globalThis Object.defineProperty allows to change the value for non-writable properties if they are configurable. We missed that case when checking if a property is read-only. Fixes: https://github.com/nodejs/node/issues/47799 --- src/node_contextify.cc | 9 ++++++--- .../test-vm-global-configurable-properties.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-vm-global-configurable-properties.js diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 23b87657cee1bb..d22ca507614b1a 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -609,11 +609,14 @@ void ContextifyContext::PropertyDefinerCallback( bool read_only = static_cast(attributes) & static_cast(PropertyAttribute::ReadOnly); + bool dont_delete = static_cast(attributes) & + static_cast(PropertyAttribute::DontDelete); - // If the property is set on the global as read_only, don't change it on - // the global or sandbox. - if (is_declared && read_only) + // If the property is set on the global as neither writable nor + // configurable, don't change it on the global or sandbox. + if (is_declared && read_only && dont_delete) { return; + } Local sandbox = ctx->sandbox(); diff --git a/test/parallel/test-vm-global-configurable-properties.js b/test/parallel/test-vm-global-configurable-properties.js new file mode 100644 index 00000000000000..4428e747eae36d --- /dev/null +++ b/test/parallel/test-vm-global-configurable-properties.js @@ -0,0 +1,15 @@ +'use strict'; +// https://github.com/nodejs/node/issues/47799 + +require('../common'); +const assert = require('assert'); +const vm = require('vm'); + +const ctx = vm.createContext(); + +const window = vm.runInContext('this', ctx); + +Object.defineProperty(window, 'x', { value: '1', configurable: true }); +assert.strictEqual(window.x, '1'); +Object.defineProperty(window, 'x', { value: '2', configurable: true }); +assert.strictEqual(window.x, '2');