Security considerations using "new Function(...)" (during rendertime, expression coming from my Javascript sources)

Question

I'd like to use new Function(...) to generate a function from very reduced code. I'l like to do this to

• avoid parsing the expression on my own and
• being as flexible as possible.

I avoid eval() whenever possible. But I'm not sure if it's secure enough to use new Function(...) which is also known as being liable to security holes.

Background

I want to manage the states of menu buttons. So, while defining the buttons, I'd like to write something like

 {
 ..., // More button definition
 state: "isInEditmode && (isWidgetSelected || isCursorInWidget),
 ...
 }

While handling the statechange during several events I'll check (summarize) the states of the current overall state object against those in the states attribute.

So I'll generate a Function during rendertime and attaching it as a DOM object attribute, not DOM attribute this way:

 ...
 $el.stateFn = new Function("stateObj", "with (stateObj) {return " + item.state + ";}");
 ...

Testing state:

 visible = $el.stateFn.call(currentStates, currentStates);

The with statement helps me providing the current state object's attributes as variables so that the above expression does not need something like obj.isInEditmode.

Security question

In my opinion this does not introduce security holes as the function attached to the DOM object is generated during render time and read from source. Or am I wrong? Should I avoid this?

Performance hints are appreciated (comment) (I think as long as I evaluating a new Function once during render time, this is acceptable).

Edit 1

• I'm using Backbone.js. Using another Framework is out of question.
• Some menu items need to be bound to different or even multiple models.
• Delegation (or facade / proxy?) models are considerable.

Answer 1

Security-wise both are just as bad if user input is allowed to break out in the code. However, maintenance wise you don't have to worry about hidden bugs when local eval messes with your scope and causes dynamic scoping.

Performance-wise the function generated by new Function is exactly the same as any other function. The generation is slower but inlike eval it doesn't cause the containing scope to be unoptimizable.

In fact, new Function can be used to improve performance in situations like:

//Will behave like function a( obj ) { return obj.something }
function makePropReader( propName ) {
    return new Function( "obj", "return obj." + propName );
}

The constructed function will perform better than the function returned here:

function makePropReader( propName ) {
     return function( obj ) {
         return obj[propName];
     }
}

Due to having to dynamically read propName from closure context and do a dynamic read on the object everytime it is called.

Answer 2

Please never ever use eval no matter what. There is a much better alternative. Instead of eval, use the Function constructor. eval is evil, and there's no question about that, but most people skip over the most evil aspect of eval: it gives you access to variables in your local scope. Back in the 90s, back before the concept of JIST compilation, eval sounded like a good idea (and it was): just insert some additional lines dynamically into the code you're already executing line-by-line. This also meant that evals didn't really slow things down all. However, now-a-days with JIST compilation eval statements are very taxing on JIST compilers which internally remove the concept of variable names entirely. For JIST compilers, in order to evaluate an eval statement, it has to figure out where all of its variables are stored, and match them with unknown globals found in the evaled statement. The problem extends even deeper if you get really technical.

But, with Function, the JIST compiler doesn't have to do any expensive variable name lookups: the entire code block is self-contained and in the global scope. For example, take the following terribly inefficient eval snippet. Please note that this is only for the purpose of being an example. In production code, you shouldn't even be using eval or Function to generate a function from a string whose content is already known.

var a = {
    prop: -1
};
var k = eval('(function(b){return a.prop + b;})');
alert( k(3) ); // will alert 2

Now, let's take a look at the much better Function alternative.

var a = {
    prop: -1
};
var k = (Function('a', 'b', 'return a.prop + b')).bind(undefined, a);
alert( k(3) ); // will alert 2

Notice the difference? There is a major one: the eval is executed inside the local scope while the Function is executed inside the global one.

Now, onto the next problem: security. There is a lot of talk about how security is difficult, and yes, with eval it is pretty much impossible (e.x. if you wrap the whole code in a sandboxing function, then all you have to do is prematurely end the function and start a new one to execute code freely in the current scope). But, with Function, you can easily (but not the most efficiently) sandbox anything. Look at the following code.

var whitelist = ['Math', 'Number', 'Object', 'Boolean', 'Array'];
var blacklist = Object.getOwnPropertyNames(window).filter(function(x){
    return whitelist.indexOf(x) === -1 && !/^[^a-zA-Z]|\W/.test(x)
});
var listlen = blacklist.length;
var blanklist = (new Array(listlen+1)).fill(undefined);
function sandboxed_function(){
    "use-strict";
    blacklist.push.apply(blacklist, arguments);
    blacklist[blacklist.length-1] = 
        '"use-strict";' + arguments[arguments.length-1];
    var newFunc = Function.apply(
        Function,
        blacklist
    );
    blacklist.length = listlen;
    return newFunc.bind.apply(newFunc, blanklist);
}

Then, fiddle around with the whitelist, get it just the way you want it, and then you can use sandboxed_function just like Function. For example:

var whitelist = ['Math', 'Number', 'Object', 'Boolean', 'Array'];
var blacklist = Object.getOwnPropertyNames(window).filter(function(x){
    return whitelist.indexOf(x) === -1 && !/^[^a-zA-Z]|\W/.test(x)
});
var listlen = blacklist.length;
var blanklist = (new Array(listlen+1)).fill(undefined);
function sandboxed_function(){
    "use-strict";
    blacklist.push.apply(blacklist, arguments);
    blacklist[blacklist.length-1] = 
        '"use-strict";' + arguments[arguments.length-1];
    var newFunc = Function.apply(
        Function,
        blacklist
    );
    blacklist.length = listlen;
    return newFunc.bind.apply(newFunc, blanklist);
}
var myfunc = sandboxed_function('return "window = " + window + "\\ndocument = " + document + "\\nBoolean = " + Boolean');
output.textContent = myfunc();

<pre id="output"></pre>

As for writing code to be runned under this strict sandbox, you may be asking, if window is undefined, how do I test for the existence of methods. There are two solutions to this. #1 is just simply to use typeof like so.

output.textContent = 'typeof foobar = ' + typeof foobar;

<div id="output"></div>

As you can see in the above code, using typeof will not throw an error, rather it will only just return undefined. The 2nd primary method to check for a global is to use the try/catch method.

try {
    if (foobar)
        output.textContent = 'foobar.constructor = ' + foobar.constructor;
    else
        output.textContent = 'foobar.constructor = undefined';
} catch(e) {
    output.textContent = 'foobar = undefined';
}

<div id="output"></div>

So, in conclusion, I hope my code snippets gave you some insight into a much better, nicer, cleaner alternative to eval. And I hope I have aspired you to a greater purpose: snubbing on eval. As for the browser compatibility, while the sandboxed_function will run in IE9, in order for it to actually sandbox anything, IE10+ is required. This is because the "use-strict" statement is very essential to eliminating much of the sneaky sand-box breaking ways like the one below.

var whitelist = ['Math', 'Number', 'Object', 'Boolean', 'Array'];
var blacklist = Object.getOwnPropertyNames(window).filter(function(x){
    return whitelist.indexOf(x) === -1 && !/^[^a-zA-Z]|\W/.test(x)
});
var listlen = blacklist.length;
var blanklist = (new Array(listlen+1)).fill(undefined);
function sandboxed_function(){
    blacklist.push.apply(blacklist, arguments);
    blacklist[blacklist.length-1] = 
        /*'"use-strict";' +*/ arguments[arguments.length-1];
    var newFunc = Function.apply(
        Function,
        blacklist
    );
    blacklist.length = listlen;
    return newFunc.bind.apply(newFunc, blanklist);
}
var myfunc = sandboxed_function(`return (function(){
    var snatched_window = this; // won't work in strict mode where the this
                                // variable doesn't need to be an object
    return snatched_window;
}).call(undefined)`);
output.textContent = "Successful broke out: " + (myfunc() === window);

<pre id="output"></pre>

One last final comment is that if you are going to allow event API's into your sandboxed environment, then you must be careful: the view property can be a window object, making it so you have to erase that too. There are several other things, but I would recommend researching thoroughly and exploring the objects in Chrome's console.

Lastly, note that Function is a very unique constructor which returns a function instead of an object instance, so there's no need to use new.

Answer 3

Old thread with answers considered dangerous these days. new Function() still allows access to global variables. So an adversary, when given the chance to effect the function string - which is usually the very reason for considering new Function and hard to guarantee it can't be done maliciously -, can read and modify any global. Good luck from that point on :-)

Which is why new Function falls under the same category as eval from the viewpoint of CSP (Content Security Policy) as mentioned here.

Example:

a = 10
> 10

b = new Function('a = 20; return 42')
> function(...)

a
> 10

b()
> 42

a
> 20

Answer 4

As you have said that you will only be doing this on code you wrote yourself - I'd say that it's fine. new Function() is definitely better than using eval() in any case. You won't be messing with any local variables and you're enforcing your own context by using fn.call.

It seems to me that the problem you are trying to solve would be fairly straight forward if you were using an MVC or MVVM framework that supports 2 way data binding. I.e. changing the UI updates a backing model and updating the model will automatically refresh the UI for you.

For example, knockout.js. In this case the visible databinding would be appropriate.