Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
contextify: dealloc only after global and sandbox
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
indutny committed Sep 12, 2013
1 parent 59dac01 commit 3d4c663
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}


Expand Down Expand Up @@ -173,7 +178,8 @@ class ContextifyContext {
static void SandboxFreeCallback(Isolate* isolate,
Persistent<Object>* target,
ContextifyContext* context) {
delete context;
if (--context->references_ == 0)
delete context;
}


Expand Down
9 changes: 9 additions & 0 deletions test/simple/test-vm-run-in-new-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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

0 comments on commit 3d4c663

Please sign in to comment.