From 3d4c663ee68326990e0732a4aa76445688e1064e Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Thu, 12 Sep 2013 17:51:55 +0400
Subject: [PATCH] contextify: dealloc only after global and sandbox

Functions created using: `vm.runInNewContext('(function() { })')` will
reference only `proxy_global_` object and not `sandbox_`. Thus in case,
where there're no references to sandbox (such as in example above),
`ContextifyContext` will be destroyed and use-after-free might happen.
---
 src/node_contextify.cc                    | 10 ++++++++--
 test/simple/test-vm-run-in-new-context.js |  9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index da636556b1d..1c9d9beb3de 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -58,15 +58,20 @@ class ContextifyContext {
   Persistent<Object> sandbox_;
   Persistent<Context> context_;
   Persistent<Object> proxy_global_;
+  int references_;
 
  public:
   explicit ContextifyContext(Environment* env, Local<Object> sandbox)
       : env_(env)
       , sandbox_(env->isolate(), sandbox)
       , context_(env->isolate(), CreateV8Context(env))
-      , proxy_global_(env->isolate(), context()->Global()) {
+      , proxy_global_(env->isolate(), context()->Global())
+        // Wait for both sandbox_'s and proxy_global_'s death
+      , references_(2) {
     sandbox_.MakeWeak(this, SandboxFreeCallback);
     sandbox_.MarkIndependent();
+    proxy_global_.MakeWeak(this, SandboxFreeCallback);
+    proxy_global_.MarkIndependent();
   }
 
 
@@ -173,7 +178,8 @@ class ContextifyContext {
   static void SandboxFreeCallback(Isolate* isolate,
                                   Persistent<Object>* target,
                                   ContextifyContext* context) {
-    delete context;
+    if (--context->references_ == 0)
+      delete context;
   }
 
 
diff --git a/test/simple/test-vm-run-in-new-context.js b/test/simple/test-vm-run-in-new-context.js
index 77b74d5e197..ecb80bdbc05 100644
--- a/test/simple/test-vm-run-in-new-context.js
+++ b/test/simple/test-vm-run-in-new-context.js
@@ -19,10 +19,14 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+// Flags: --expose-gc
+
 var common = require('../common');
 var assert = require('assert');
 var vm = require('vm');
 
+assert.equal(typeof gc, 'function', 'Run this test with --expose-gc');
+
 common.globalCheck = false;
 
 console.error('run a string');
@@ -60,3 +64,8 @@ var f = { a: 1 };
 vm.runInNewContext('f.a = 2', { f: f });
 assert.equal(f.a, 2);
 
+console.error('use function in context without referencing context');
+var fn = vm.runInNewContext('(function() { obj.p = {}; })', { obj: {} })
+gc();
+fn();
+// Should not crash