Skip to content

Commit 6603d32

Browse files
fhinkeltargos
authored andcommitted
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: #47799 PR-URL: #51602 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent c820166 commit 6603d32

File tree

2 files changed

+21
-3
lines changed

2 files changed

+21
-3
lines changed

src/node_contextify.cc

+6-3
Original file line numberDiff line numberDiff line change
@@ -609,11 +609,14 @@ void ContextifyContext::PropertyDefinerCallback(
609609
bool read_only =
610610
static_cast<int>(attributes) &
611611
static_cast<int>(PropertyAttribute::ReadOnly);
612+
bool dont_delete = static_cast<int>(attributes) &
613+
static_cast<int>(PropertyAttribute::DontDelete);
612614

613-
// If the property is set on the global as read_only, don't change it on
614-
// the global or sandbox.
615-
if (is_declared && read_only)
615+
// If the property is set on the global as neither writable nor
616+
// configurable, don't change it on the global or sandbox.
617+
if (is_declared && read_only && dont_delete) {
616618
return;
619+
}
617620

618621
Local<Object> sandbox = ctx->sandbox();
619622

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use strict';
2+
// https://github.com/nodejs/node/issues/47799
3+
4+
require('../common');
5+
const assert = require('assert');
6+
const vm = require('vm');
7+
8+
const ctx = vm.createContext();
9+
10+
const window = vm.runInContext('this', ctx);
11+
12+
Object.defineProperty(window, 'x', { value: '1', configurable: true });
13+
assert.strictEqual(window.x, '1');
14+
Object.defineProperty(window, 'x', { value: '2', configurable: true });
15+
assert.strictEqual(window.x, '2');

0 commit comments

Comments
 (0)