Making WebWorkers a safe environment

Question

In a quest to have an interface capable of running arbitrary javascript code inside the browser, without having a security hole the size of a typical yo-mama joke, Esailija proposed using Web Workers. They run in a semi-sandboxed environment (no DOM access and already inside the browser) and can be killed so the user can't put them in an infinite loop.

Here's the example he brought up: http://tuohiniemi.fi/~runeli/petka/workertest.html (open your console)

jsfiddle (Google chrome only)

Now, this seems like a good solution; however, is it a complete (or approaching complete) one? Is there anything obvious missing?

The entire thing (as it's hooked up to a bot) can be found on github: worker, evaluator

main:

workercode = "worker.js";

function makeWorkerExecuteSomeCode( code, callback ) {
    var timeout;

    code = code + "";
    var worker = new Worker( workercode );

    worker.addEventListener( "message", function(event) {
        clearTimeout(timeout);
        callback( event.data );
    });

    worker.postMessage({
        code: code
    });

    timeout = window.setTimeout( function() {
        callback( "Maximum execution time exceeded" );
        worker.terminate();
    }, 1000 );
}

makeWorkerExecuteSomeCode( '5 + 5', function(answer){
    console.log( answer );
});

makeWorkerExecuteSomeCode( 'while(true);', function(answer){
    console.log( answer );
});

var kertoma = 'function kertoma(n){return n === 1 ? 1 : n * kertoma(n-1)}; kertoma(15);';

makeWorkerExecuteSomeCode( kertoma, function(answer){
    console.log( answer );
});

worker:

var global = this;

/* Could possibly create some helper functions here so they are always available when executing code in chat?*/

/* Most extra functions could be possibly unsafe */

    var wl = {
        "self": 1,
        "onmessage": 1,
        "postMessage": 1,
        "global": 1,
        "wl": 1,
        "eval": 1,
        "Array": 1,
        "Boolean": 1,
        "Date": 1,
        "Function": 1,
        "Number" : 1,
        "Object": 1,
        "RegExp": 1,
        "String": 1,
        "Error": 1,
        "EvalError": 1,
        "RangeError": 1,
        "ReferenceError": 1,
        "SyntaxError": 1,
        "TypeError": 1,
        "URIError": 1,
        "decodeURI": 1,
        "decodeURIComponent": 1,
        "encodeURI": 1,
        "encodeURIComponent": 1,
        "isFinite": 1,
        "isNaN": 1,
        "parseFloat": 1,
        "parseInt": 1,
        "Infinity": 1,
        "JSON": 1,
        "Math": 1,
        "NaN": 1,
        "undefined": 1
    };

    Object.getOwnPropertyNames( global ).forEach( function( prop ) {
        if( !wl.hasOwnProperty( prop ) ) {
            Object.defineProperty( global, prop, {
                get : function() {
                    throw new Error( "Security Exception: cannot access "+prop);
                    return 1;
                }, 
                configurable : false
            });    
        }
    });

    Object.getOwnPropertyNames( global.__proto__ ).forEach( function( prop ) {
        if( !wl.hasOwnProperty( prop ) ) {
            Object.defineProperty( global.__proto__, prop, {
                get : function() {
                    throw new Error( "Security Exception: cannot access "+prop);
                    return 1;
                }, 
                configurable : false
            });    
        }
    });




onmessage = function( event ) {
    "use strict";
    var code = event.data.code;
    var result;
    try {
        result = eval( '"use strict";\n'+code );
    }
    catch(e){
        result = e.toString();
    }
    postMessage( "(" + typeof result + ")" + " " + result );
};

@SLaks You can manually remove a bunch of insecure properties yourselves, as shown in the example. — Zirak, May 18 '12 at 13:46
It appears that if you `delete` a native/host object, it will be restored in its original state. `"delete XMLHttpRequest; XMLHttpRequest;"` Will return the original XMLHttpRequest object. There must be a way around this :/ — Esailija, May 18 '12 at 14:08
Frankly, this is brilliant. I wish I could give you more rep for this. — Snuffleupagus, May 18 '12 at 14:47
You have the blacklist of functions, that are unsafe. It doesn't look good. What if new standard or browser defines new unsafe functions? — zch, May 18 '12 at 15:43
I think you need whitelist - remove all functions, except those that you know they are safe. — zch, May 18 '12 at 15:56
@zch I just noticed this when I enumerated over global object in worker. I am making a whitelist approach. Thanks — Esailija, May 18 '12 at 15:57
As a paranoid I would add some kind of runtime test, that your method actually blocks something on working system. Otherwise, looks okay, but I'm not an expert. — zch, May 18 '12 at 21:08
How about using something like: http://code.google.com/p/google-caja/ ? — Ido Green, May 23 '12 at 06:32
Maybe running a VM such as [continuum](https://github.com/Benvie/continuum) could be an idea? — Florian Margaine, May 31 '13 at 17:35
@IdoGreen That'd require incorporating a huge outside framework. A native solution is preferred. — Zirak, May 31 '13 at 19:51
@FlorianMargaine That's learning to the preferable, but comes with a huge performance kill — Zirak, May 31 '13 at 19:52
What exact workaround was added to prevent delete keyword from undoing it? @Esailija — Gregory Magarshak, Dec 25 '18 at 22:30

score 40 · Accepted Answer · edited Apr 11 '14 at 09:32

40

The current code (listed below) has been now in use in the Stackoverflow javascript chat room for a while and so far the toughest problem was Array(5000000000).join("adasdadadasd") instantly crashing some browser tabs for me when I was running the code executor bot. Monkeypatching Array.prototype.join seems to have fixed this and the maximum execution time of 50ms has worked for any other attempt to hog memory or crash the browser.

var global = this;

/* Could possibly create some helper functions here so they are always available when executing code in chat?*/

/* Most extra functions could be possibly unsafe */

var wl = {
    "self": 1,
    "onmessage": 1,
    "postMessage": 1,
    "global": 1,
    "wl": 1,
    "eval": 1,
    "Array": 1,
    "Boolean": 1,
    "Date": 1,
    "Function": 1,
    "Number" : 1,
    "Object": 1,
    "RegExp": 1,
    "String": 1,
    "Error": 1,
    "EvalError": 1,
    "RangeError": 1,
    "ReferenceError": 1,
    "SyntaxError": 1,
    "TypeError": 1,
    "URIError": 1,
    "decodeURI": 1,
    "decodeURIComponent": 1,
    "encodeURI": 1,
    "encodeURIComponent": 1,
    "isFinite": 1,
    "isNaN": 1,
    "parseFloat": 1,
    "parseInt": 1,
    "Infinity": 1,
    "JSON": 1,
    "Math": 1,
    "NaN": 1,
    "undefined": 1
};

Object.getOwnPropertyNames( global ).forEach( function( prop ) {
    if( !wl.hasOwnProperty( prop ) ) {
        Object.defineProperty( global, prop, {
            get : function() {
                throw "Security Exception: cannot access "+prop;
                return 1;
            }, 
            configurable : false
        });    
    }
});

Object.getOwnPropertyNames( global.__proto__ ).forEach( function( prop ) {
    if( !wl.hasOwnProperty( prop ) ) {
        Object.defineProperty( global.__proto__, prop, {
            get : function() {
                throw "Security Exception: cannot access "+prop;
                return 1;
            }, 
            configurable : false
        });    
    }
});

Object.defineProperty( Array.prototype, "join", {

    writable: false,
    configurable: false,
    enumerable: false,

    value: function(old){
        return function(arg){
            if( this.length > 500 || (arg && arg.length > 500 ) ) {
                throw "Exception: too many items";
            }

            return old.apply( this, arguments );
        };
    }(Array.prototype.join)

});


(function(){
    var cvalues = [];

    var console = {
        log: function(){
            cvalues = cvalues.concat( [].slice.call( arguments ) );
        }
    };

    function objToResult( obj ) {
        var result = obj;
        switch( typeof result ) {
            case "string":
                return '"' + result + '"';
                break;
            case "number":
            case "boolean":
            case "undefined":
            case "null":
            case "function":
                return result + "";
                break;
            case "object":
                if( !result ) {
                    return "null";
                }
                else if( result.constructor === Object || result.constructor === Array ) {
                    var type = ({}).toString.call( result );
                    var stringified;
                    try {
                        stringified = JSON.stringify(result);
                    }
                    catch(e) {
                        return ""+e;
                    }
                    return type + " " + stringified;
                }
                else {
                    return ({}).toString.call( result );
                }
                break;

        }

    }

    onmessage = function( event ) {
        "use strict";
        var code = event.data.code;
        var result;
        try {
            result = eval( '"use strict";\n'+code );
        }
        catch(e) {
            postMessage( e.toString() );
            return;
        }
        result = objToResult( result );
        if( cvalues && cvalues.length ) {
            result = result + cvalues.map( function( value, index ) {
                return "Console log "+(index+1)+":" + objToResult(value);
            }).join(" ");
        }
        postMessage( (""+result).substr(0,400) );
    };

})();

edited Apr 11 '14 at 09:32

Domi

22,151
15
92
122

answered May 29 '12 at 09:42

Esailija

138,174
23
272
326

Why are Math, and parse* functions disabled? – Domi Apr 10 '14 at 13:48
@Domi why do you think they are disabled – Esailija Apr 10 '14 at 13:51
@Esailija So far, I only found [cross-domain Math.random() prediction](http://ifsec.blogspot.tw/2012/05/cross-domain-mathrandom-prediction.html). But since the attacker cannot communicate to the outside world, it seems to be harmless. – Domi Apr 10 '14 at 14:06
Btw, I am also disallowing `XMLHttpRequest`, `Worker`, `WebSocket` and `importScripts`. How do you guys cope with that? – Domi Apr 10 '14 at 14:09
1

@Domi I think you are misunderstanding - this is a whitelist approach. Anything not listed is disallowed while only the things that are listed are allowed. Whitelist is better because it doesn't require updating when new APIs are implemented. – Esailija Apr 10 '14 at 14:42
What would happen if any of the insecure properties are already non-configurable? – Bergi Apr 10 '14 at 15:28
@Bergi that is not happening in the use case – Esailija Apr 10 '14 at 16:52
2

How do you know that for future APIs? :-) But I've checked it now, `Object.defineProperty` would throw and prevent the `onmessage` handler from being set up. – Bergi Apr 10 '14 at 17:04
I tried this in Chrome, and it reported "cannot redefine property [Intl](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl)". Adding it to the whitelist fixed it. – Domi Apr 11 '14 at 09:01
One more stupid question: What is dangerous about allowing `console`? Is that also part of making it "future-proof", or can it currently be exploited? – Domi Apr 11 '14 at 09:03
@Domi I don't think there is a `console` in a web worker – Esailija Apr 11 '14 at 11:10
@Esailija there is, at least on Chrome and IE. Been using it for quite a while already. I am guessing that it is running on its own thread. – Domi Apr 11 '14 at 20:36
@Esailija bad news: There is a Chrome bug (or feature?) where a whole bunch of globals are not listed by `Object.getOwnPropertyNames`, including all *Timeout and *Interval functions. [This JSFiddle](http://jsfiddle.net/WuhrP/) shows a rather long list of such globals (at least in Chrome). This script won't run in IE because it uses in-line Workers (The current IE version disallows running workers from `Blob` URLs, for some stupid reason). – Domi Apr 13 '14 at 15:42
@Domi that's not a bug and those are found by `Object.getOwnPropertyNames( global.__proto__ )` – Esailija Apr 14 '14 at 10:24
@Esailija That is not true. `global.__proto__` only finds two additional globals in my Chrome: `constructor` and `postMessage`. The rest is still missing, including *Timeout and *Interval functions. [See this updated JSFiddle](http://jsfiddle.net/WuhrP/1/) for proof. – Domi Apr 14 '14 at 11:25
1

@Domi *Timeout and *Interval function are listed http://jsfiddle.net/WuhrP/2/ (I am using version 33.0.1750.152) – Esailija Apr 14 '14 at 12:14
@Esailija Apologies, my code probably had a bug then. Anyway, would you mind taking a look at [this related question](http://stackoverflow.com/questions/23078059/is-it-possible-to-read-local-variables-of-parent-stackframes)? – Domi Apr 15 '14 at 08:40
@Esailija The code in your answer looks like the one in the question. You might want to check out [this answer](http://stackoverflow.com/a/26798249/1906307). – Louis Nov 07 '14 at 09:41
Is there any way for code in the WebWorker to undo the disabling, eg with delete keyword? – Gregory Magarshak Dec 25 '18 at 22:32
Hi @Esailija, are you still using this code in stackoverflow chat? – Artur Carvalho Aug 03 '21 at 14:30

Louis · Answer 2 · 2014-11-07T14:28:16.490

The code currently (2014-11-07) shown in the question despite ostensibly disallowing access to XMLHttpRequest (since it is not whitelisted), still allows code to access it.

If I put the code in the question (or the accepted answer) in a web page and worker combo and execute the following code on Chrome 38:

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

The result is:

function XMLHttpRequest() { [native code] }

However it does not work in FF. Bug in Chrome?

Another thing I found but which does not seem to lead very far down the rabit hole is reinstating console.log. This works on FF 31 but not Chrome 38:

makeWorkerExecuteSomeCode(
    'var c = self.__proto__.__proto__.__lookupGetter__("console").call(self); c.log("FOO");', 
    function (answer) { console.log(answer) });

This would log "FOO" to the console without passing through the fake console.log that the web worker provides. The code above uses self, which can be blacklisted (by removing it from the whitelist) but this and global also work. I've found that attempts to blacklist global fail on FF and Chrome: the worker dies with an error.

Note: Chrome refuses to blacklist Intl so it has to be added to the whitelist for the code to run at all.

Very good point concerning the hole in Chrome 38, but that is rather easy to fill: Just put a closure around the `try/catch` in `onmessage` and redefine `event` with `var event;` within the closure. — heinob, Nov 07 '14 at 14:05

Making WebWorkers a safe environment

2 Answers2

Linked