30
\$\begingroup\$

In a large project of mine, I've run into a situation where a client might need to run evaluated JavaScript code. I know, it makes me cringe too. One option is to manually parse it, but for future flexibility, I would like to evaluate the code safely.

Everything I've seen and researched says, NO. Ah, the nay-sayers.

This is my attempt to sandbox the eval:

jsFiddle

Can you shoot holes in it and try and break it? I should wrap it in a try so it doesn't throw ugly errors, but beyond that, tell me where I have gone awry.

/***************************************
 * Senica Gonzalez ([email protected])
 * This is an attempt to sandbox an eval
 * in javascript using an iframe.
 * Test it and let me know if you
 * can break it.
 ***************************************/

window.addEventListener("message", function(event){
    $('#result').text(event.data.eval);
    console.log(event.data.scope1);
    console.log(event.data.scope2);
}, false);

test = 'do I exist in the window?';

var input = $('[name=toeval]');
input.on('change', function(){
    var val = $(this).val();
    var code = btoa('<html>\
        <head><\/head>\
        <body>\
            <' + 'script>\
                var party_size = 10;\
                window.parent.postMessage({\
                    eval: eval('+val+'),\
                }, "*");\
            <\/script' + '>\
        <\/body>\
    <\/html>');
    var frame = $('<iframe sandbox="allow-scripts" src="data:text/html;base64,'+code+'"></iframe>');
    var sandbox = $('#sandbox');
    sandbox.html(frame);
});

I have been unsuccessful at accessing the parent document variables, or being able to do anything obnoxious other than to my self within the sandbox.

Improve this question
\$\endgroup\$
9
  • \$\begingroup\$ The character sequence "</script>" should be escaped using "<\/script>" (and so should all other closing tags in string literals), the way you have it does nothing useful. \$\endgroup\$
    – RobG
    May 1, 2014 at 0:28
  • \$\begingroup\$ You didn't run it. It works as is. If you escape them, then it won't run as a script, which it needs to. \$\endgroup\$
    – Senica Gonzalez
    May 1, 2014 at 3:28
  • \$\begingroup\$ allow-same-origin is all the security hole you need, isn't it? Also what about browsers that don't understand the sandbox attribute? \$\endgroup\$
    – Bergi
    May 1, 2014 at 11:32
  • \$\begingroup\$ @SenicaGonzalez—it runs because browsers ignore the script tag that should close the element (test it in an HTML validator). Quoting the closing / as suggested certainly does not stop the inserted script from running. See Why split the <script> tag when writing it with document.write()?. \$\endgroup\$
    – RobG
    May 1, 2014 at 13:28
  • 2
    \$\begingroup\$ Read - stackoverflow.com/questions/16681460/… , also related stackoverflow.com/questions/10653809/… and of course - check out "Secure ECMAScript" \$\endgroup\$
    – Benjamin Gruenbaum
    May 1, 2014 at 14:43

3 Answers 3

Reset to default
13
\$\begingroup\$

Yes, I can poke a few holes in this.

Firstly, your eval() isn't actually doing much of anything here. From the perspective of the parser running the script, that line looks like this:

eval(party_size < 5)

This means that party_size < 5 is evaluated to false before it is passed to the eval() function. eval() then just spits this value right back out.

Some consequences of this:

  • It will fail to act as expected if the expression evaluates to a string value. For example, if you put "a" + "b" into your textbox, this will fail with the error ReferenceError: ab is not defined, because the JS engine will be trying to evaluate eval("ab").
  • It will fail if the code is anything other than a single expression. So entering this into the textbox: var a = 2; a; fails with a syntax error, even though this is a perfectly legitimate thing to pass to eval().


So let's assume that was just a silly oversight and you meant to do this:

eval: eval("'+val+'"),\

Well, this will succeed in actually executing chunks of code, until that code contains a quotation mark:

eval("var a = "a"; a + "b"");

at which point it will be broken again.


Now let's imagine you decided to get around this by changing this line to:

eval: eval("' + val.replace(/"/g, '\\"') + '"),\

Now you can be pretty confident that the code will run (in most cases) and that all you need to worry about at this point is malicious code (haha).

In terms of malicious code, the attacker could execute something resource intensive that hogs the CPU and RAM:

var a = []; while(true) { a.push(new Date()); console.log("gotcha!"); }

Or they might choose to carry out a CSRF-like attack:

var x = new XMLHttpRequest();
x.open("post", "https://www.mybank.com/transferMoney");
x.send("amount=10000&toAccount=34567890");

So keeping the host DOM safe is really not the only thing to worry about. There are any other number of malicious things an attacker can carry out if they're able to run code on someone's machine.

Given the above, I would stick with the assertion that eval is evil, and it's not going to be easy to find a "safe" way to execute untrusted code.

Improve this answer
\$\endgroup\$
1
6
\$\begingroup\$

There is a safe and sandboxed way to execute JavaScript code without using eval, is to have a JavaScript interpreter written in JavaScript itself. There is an implementation by Neil Fraser at Google called JS-Interpreter, I am using it and it is a good and simple solution, this is the repo in Github. Hope it helps.

Improve this answer
\$\endgroup\$
0
\$\begingroup\$

A possible solution for Node.js and the browser is:

var globalScope = global ? global : window // node.js and browser
var globals = Object.getOwnPropertyNames(globalScope)

module.exports = function makeSafeEval (include) {
  var clearGlobals = ''
  for (var i = 0, len = globals.length; i < len; i++) {
    if (include && include.indexOf(globals[i]) === -1 || !include) {
      clearGlobals += 'var ' + globals[i] + ' = undefined;'
    }
  }
  return function (operation) {
    var globals = undefined // out of scope for operation
    return eval('(function () {' + clearGlobals + ';return ' + operation.replace('this', '_this') + '})()')
  }
}

var safeEval = makeSafeEval()
safeEval('this') // undefined
safeEval('window') // undefined

This not prevents you from malicius code that consumes the memory and CPU. This protection would be achieved by executing the above code in a worker and send messages in an interval, if blocked you can terminate it. On browser you can use a Webworker and in node a Cluster Worker. The above code is avalaible as npm module as cross-safe-eval and the code in my Github, hope it helps someone.

Improve this answer
\$\endgroup\$
1
  • 4
    \$\begingroup\$ This is NOT a safe solution!! It's possible to prevent the function which clears globals from running (in a mysql-injection-like fashion). If operation is set to: 0; }); (function() { console.log('Now I can do anything!!'); var thiis = eval('th' + 'is'); then a malicious user can run any code they like, and even get a reference to this. This is because the global-clearing code is parsed into a function which is never called. \$\endgroup\$
    – Gershy
    Oct 19, 2017 at 19:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.