Securing JavaScript eval function

Question

We want to give our users the ability to execute self created JavaScript code within our application. For this we need to use eval to evaluate the code. To reduce all security concerns to a minimum (if not zero), our idea is to prevent the usage of any window or document function within the code. So no XMLHttpRequest or anything similar.

This is the code:

function secure_eval(s) {
    var ret;

    (function(){
        var copyXMLHttpRequest = XMLHttpRequest; // save orginal function in copy

        XMLHttpRequest = undefined; // make orignal function unavailable

        (function() {
            var copyXMLHttpRequest; // prevent access to copy

            try {
                ret = eval(s)
            } catch(e) {
                console.log("syntax error or illegal function used");
            }

        }())
        XMLHttpRequest = copyXMLHttpRequest; // restore original function
    }())
    return ret;
}

This works as follows:

secure_eval('new XMLHttpRequest()'); // ==> "illegal function used"

Now I have several questions:

1. Is this pattern the right way to secure eval?
2. What functions of window and document are the ones which are considered harmful?
3. To ship around question 2. I tried to mask all (native) functions of window But I am not able to enumerate them:

This does not list XMLHttpRequest for instance:

for( var x in window) {
    if( window[x] instanceof Function) {
        console.log(x);
    }
}

Is there a way to get a list of all native functions of window and document?

EDIT:

One of my ideas is to perform the eval within a Worker and prevent access to XMLHttpRequest and document.createElement (see my solution above). This would have (to my mind) the following consequences:

• no access to the original document
• no access to the original window
• no chance to communicate with external resources (no ajax, no scripts)

Do you see any drawback or leaks here?

EDIT2:

In the meantime I have found this question which answer solves many of my problems plus a couple of things I did not even think about (i.e. browser dead lock with "while(true){}".

Answer 1

Your code does not actually prevent the use of XMLHttpRequest. I can instantiate an XMLHttpRequest object with these methods:

secure_eval("secure_eval = eval"); // Yep, this completely overwrites secure_eval.
secure_eval("XMLHttpRequest()");

Or:

secure_eval("new (window.open().XMLHttpRequest)()")

Or:

secure_eval("new (document.getElementById('frame').contentWindow.XMLHttpRequest)()")

This 3rd method relies on the presence of an iframe in the HTML of the page, which someone could add by manipulating the DOM in their browser. I do such manipulations every now and then with Greasemonkey to remove annoyances or fix broken GUIs.

This took me about 5 minutes to figure out, and I am not by any means a security guru. And these are only the holes I was able to find quickly, there are probably others, that I don't know about. The lesson here is that it is really really really hard to secure code through eval.

Using A Worker

Ok, so using a Worker to run the code is going to take care of the 2nd and 3rd cases above because there's no window accessible in a Worker. And... hmm.. the 1st case can be handled by shadowing secure_eval inside its scope. End of story? If only...

If I put secure_eval inside a web worker and run the following code, I can reacquire XMLHttpRequest:

secure_eval("var old_log = console.log; console.log = function () { foo = XMLHttpRequest; old_log.apply(this, arguments); };");
console.log("blah");
console.log(secure_eval("foo"));

The principle is to override a function that is used outside secure_eval to capture XMLHttpRequest by assigning it to a variable that will be deliberately leaked to the global space of the worker, wait until that function is used by the worker outside secure_eval, and then grab the saved value. The first console.log above simulates the use of the tampered function outside secure_eval and the 2nd console.log shows that the value was captured. I've used console.log because why not? But really any function in the global space could be modified like this.

Actually, why wait until the worker may use some function we tampered with? Here's another, better, quicker way to do access XMLHttpRequest:

secure_eval("setTimeout(function () { console.log(XMLHttpRequest);}, 0);");

Even in a worker (with a pristine console.log), this will output the actual value of XMLHttpRequest to the console. I'll also note that the value of this inside the function passed to setTimeout is the global scope object (i.e. window when not in a worker, or self in a worker), unaffected by any variable shadowing.

What About the Other Question Mentioned in This Question?

What about the solution here? Much much better but there is still a hole when run in Chrome 38:

makeWorkerExecuteSomeCode('event.target.XMLHttpRequest', 
    function (answer) { console.log( answer ); });

This will show:

function XMLHttpRequest() { [native code] }

Again, I'm no security guru or cracker bent on causing trouble. There are probably still more ways I'm not thinking about.

Answer 2

I'll try and answer your questions in order here.

Is this pattern the right way to secure eval?

This part is slightly subjective. I don't see any major security drawbacks to this. I tried several ways to access XMLHttpRequest, but i couldn't:

secure_eval('XMLHttpRequest')
secure_eval('window.XMLHttpRequest')
secure_eval('eval("XMLHttpRequest")()')
secure_eval('window.__proto__.XMLHttpRequest') // nope, it's not inherited

However, it will be a lot if you want to blacklist more things.

What functions of window and document are the ones which are considered harmful?

That depends on what you consider "harmful". Is it bad if the DOM is accessible at all? Or what about WebKit desktop notifications, or speech synthesis?

You'll have to decide this based on your specific use case.

To ship around question 2. I tried to mask all (native) functions of window, but I am not able to enumerate them:

That's because most of the methods are non-enumerable. To enumerate, you can use Object.getOwnPropertyNames(window):

var globals = Object.getOwnPropertyNames(window);
for (var i = 0; i < globals.length; i++) {
    if( window[globals[i]] instanceof Function) {
        console.log(globals[i]);
    }
}

One of my ideas is to perform the eval within a Worker and prevent access to XMLHttpRequest and document.createElement (see my solution above).

This sounds like a good idea.