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

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants

thejh commented Sep 13, 2011

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 commented Sep 13, 2011

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

Owner

bnoordhuis commented Sep 13, 2011

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 commented Sep 13, 2011

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 commented Sep 13, 2011

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 commented Sep 13, 2011

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.

Owner

bnoordhuis commented Sep 14, 2011

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.

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

Should be closed?

Owner

bnoordhuis commented Nov 24, 2012

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 closed this Nov 24, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment