src: fix vm module for strict mode

This patch fixes the problem with variables that
are declared only on the sandbox but not on the
global proxy.

PR-URL: https://github.com/nodejs/node/pull/16487
Fixes: https://github.com/nodejs/node/issues/12300
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

1 files changed, 19 insertions, 2 deletions

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index fbc04ba0c0..12edf20810 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -346,14 +346,21 @@ class ContextifyContext {
       return;
 
     auto attributes = PropertyAttribute::None;
-    bool is_declared = ctx->global_proxy()
+    bool is_declared_on_global_proxy = ctx->global_proxy()
         ->GetRealNamedPropertyAttributes(ctx->context(), property)
         .To(&attributes);
     bool read_only =
         static_cast<int>(attributes) &
         static_cast<int>(PropertyAttribute::ReadOnly);
 
-    if (is_declared && read_only)
+    bool is_declared_on_sandbox = ctx->sandbox()
+        ->GetRealNamedPropertyAttributes(ctx->context(), property)
+        .To(&attributes);
+    read_only = read_only ||
+        (static_cast<int>(attributes) &
+        static_cast<int>(PropertyAttribute::ReadOnly));
+
+    if (read_only)
       return;
 
     // true for x = 5
@@ -371,10 +378,20 @@ class ContextifyContext {
     // this.f = function() {}, is_contextual_store = false.
     bool is_function = value->IsFunction();
 
+    bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
     if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
         !is_function)
       return;
 
+    if (!is_declared_on_global_proxy && is_declared_on_sandbox  &&
+        args.ShouldThrowOnError() && is_contextual_store && !is_function) {
+      // The property exists on the sandbox but not on the global
+      // proxy. Setting it would throw because we are in strict mode.
+      // Don't attempt to set it by signaling that the call was
+      // intercepted. Only change the value on the sandbox.
+      args.GetReturnValue().Set(false);
+    }
+
     ctx->sandbox()->Set(property, value);
   }