From a349ca85b6294bd530458f2f4b189ecf3e9bd23c Mon Sep 17 00:00:00 2001
From: Franziska Hinkelmann <franzih@chromium.org>
Date: Thu, 28 Jul 2016 11:52:16 +0200
Subject: [PATCH] src: do not copy on failing setProperty()

In vm, the setter interceptor should not copy a value onto the
sandbox, if setting it on the global object will fail. It will fail if
we are in strict mode and set a value without declaring it.

Fixes https://github.com/nodejs/node/issues/5344
---
 src/node_contextify.cc               | 14 +++++++++++++-
 test/parallel/test-vm-strict-mode.js | 27 +++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-vm-strict-mode.js

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 7ec03c10f7743a..95911be7e708e6 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -380,7 +380,19 @@ class ContextifyContext {
     if (ctx->context_.IsEmpty())
       return;
 
-    ctx->sandbox()->Set(property, value);
+    bool is_declared =
+        ctx->global_proxy()->HasRealNamedProperty(ctx->context(),
+                                                  property).FromJust();
+    bool is_contextual_store = ctx->global_proxy() != args.This();
+
+    bool set_property_will_throw =
+        args.ShouldThrowOnError() &&
+        !is_declared &&
+        is_contextual_store;
+
+    if (!set_property_will_throw) {
+      ctx->sandbox()->Set(property, value);
+    }
   }
 
 
diff --git a/test/parallel/test-vm-strict-mode.js b/test/parallel/test-vm-strict-mode.js
new file mode 100644
index 00000000000000..f3820d8aead6af
--- /dev/null
+++ b/test/parallel/test-vm-strict-mode.js
@@ -0,0 +1,27 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+const vm = require('vm');
+const ctx = vm.createContext();
+
+// Test strict mode inside a vm script, i.e., using an undefined variable
+// throws a ReferenceError. Also check that variables
+// that are not successfully set in the vm, must not be set
+// on the sandboxed context.
+
+vm.runInContext('w = 1;', ctx);
+assert.strictEqual(1, ctx.w);
+
+assert.throws(function() { vm.runInContext('"use strict"; x = 1;', ctx); },
+              /ReferenceError: x is not defined/);
+assert.strictEqual(undefined, ctx.x);
+
+vm.runInContext('"use strict"; var y = 1;', ctx);
+assert.strictEqual(1, ctx.y);
+
+vm.runInContext('"use strict"; this.z = 1;', ctx);
+assert.strictEqual(1, ctx.z);
+
+// w has been defined
+vm.runInContext('"use strict"; w = 2;', ctx);
+assert.strictEqual(2, ctx.w);