Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

add a safer (and slower, and worse) runInNewContext (see #1469) #1696

Closed
wants to merge 1 commit into from

5 participants

@thejh

This adds a function vm.runInNewSafeContext that takes the same arguments as vm.runInNewContext but is more secure:

  • it prevents crawling up the caller chain using recursion
  • it deep-clones everything in the global object and replaces functions with wrapper functions so that console.log.constructor is the Function from the eval-context, not the one from the global context (new Function(code) constructs a function that behaves like it was run in the context the Function thing comes from, so it's like a leaked eval)

I didn't just put this in vm.runInNewContext because some people maybe won't like the auto-deep-clone.

> vm.runInNewSafeContext('function f(){return f.caller===f.caller.caller};f()',{print:console.log})
true
> vm.runInNewContext('print.constructor',{print:function(){}})===Function
true
> vm.runInNewSafeContext('print.constructor',{print:function(){}})===Function
false
@thejh

Mhm, that deep clone thing throws a Maximum call stack size exceeded error on self-referencing stuff, I think I should fix that.

@bnoordhuis

runInNewSafeContext may create the wrong expectations. Can you prove that it's impossible to break out of or stop outside information from leaking in?

@thejh

Well, I guess proving it is very hard. I'm pretty certain, but you're right, that's probably not enough for "safe". What do you think, what would be an appropriate name? runSaferInNewContext?

@skang

In the middle of building an application with Node where user can write server-side javascript in their own application - I'm in despair need of this new functionality, which seems to be what I need?

It would be super if we can run a piece of server-side javascript in a "web worker" thread (or process?) and still communicate with the calling script async-ly with events/listeners. But again - "SAFE" is the key requirement - a user script shouldn't be able to call anything more than what's exposed to it's context.

@bmeck

Beware of type coersion attacks, and getter / setter attacks. You can test against something like (not the most complete though) : https://gist.github.com/1214975 . This test will hurt feelings and is stupidly hard to lock down.

@bnoordhuis

I'd like some of the other team members to chime in but I personally don't see this working out. VM contexts were never meant to be secure, it seems like a battle you can't win.

@trevnorris
Owner

Looks like @bnoordhuis thinks it's a no-go, and as @bmeck stated in #1469, use the "sandbox" npm module.

Should be closed?

@bnoordhuis

Should be closed?

Yes. I appreciate the initiative but we're a year down the road and I still don't think it's going to work. :-)

@bnoordhuis bnoordhuis closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 45 additions and 0 deletions.
  1. +45 −0 lib/vm.js
View
45 lib/vm.js
@@ -30,3 +30,48 @@ exports.createContext = binding.NodeScript.createContext;
exports.runInContext = binding.NodeScript.runInContext;
exports.runInThisContext = binding.NodeScript.runInThisContext;
exports.runInNewContext = binding.NodeScript.runInNewContext;
+
+exports.runInNewSafeContext = function(code, context) {
+ var middleContext = {};
+ middleContext.middleContext = middleContext;
+ middleContext.subGlobal = context;
+ middleContext.g_code = code;
+ middleContext.runInNewContext = binding.NodeScript.runInNewContext;
+ var copyingClosure = (function() {
+ function deepClone(obj) {
+ var type = typeof obj;
+ if (type==='undefined' || obj===null || type==='boolean' || type==='number' || type==='string') {
+ return obj;
+ }
+ if (type==='function') {
+ return function() {
+ obj.apply(this, arguments);
+ };
+ }
+ if (type==='object') {
+ var toReturn = Array.isArray(obj) ? [] : {};
+ var objKeys = Object.getOwnPropertyNames(obj);
+ for (var i=0; i<objKeys.length; i++) {
+ toReturn[objKeys[i]] = deepClone(obj[objKeys[i]]);
+ }
+ return toReturn;
+ }
+ throw new Error('unknown typeof result: "'+type+'"');
+ }
+ subGlobal = deepClone(subGlobal);
+ }).toString();
+ var newCode = "("+copyingClosure+")();"
+ var protectiveRecursion = (function() {
+ if (!this || !this.goOn) {
+ return arguments.callee.call({goOn: true});
+ }
+ var code = g_code;
+ delete middleContext.g_code;
+ var run = runInNewContext;
+ delete middleContext.runInNewContext;
+ delete middleContext.middleContext;
+ return run(code, subGlobal);
+ }).toString();
+ newCode += "("+protectiveRecursion+")()";
+ return binding.NodeScript.runInNewContext(newCode, middleContext);
+}
Something went wrong with that request. Please try again.