Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not working for web workers #90

Closed
ecofi opened this issue Mar 12, 2014 · 25 comments
Closed

Not working for web workers #90

ecofi opened this issue Mar 12, 2014 · 25 comments

Comments

@ecofi
Copy link

ecofi commented Mar 12, 2014

Hi,

the project is interesting and I was looking for a way to get the script running within a web worker. Chrome browser is blocking because of security issues. I guess it is the use of an eval function. The error message is:

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' chrome-extension-resource:". acorn.js line 397.

Would it be possible to look into this issue and replace the eval function? Would like to acorn.js.

Thanks
hle

@marijnh
Copy link
Member

marijnh commented Mar 12, 2014

I have successfully ran a Tern server in a Web worker, also in Chrome. And I have never heard of eval being forbidden in Web workers (such a restriction wouldn't make any sense). Can you show a minimal example that reproduces this problem?

@getify
Copy link

getify commented Mar 12, 2014

If a server sends out a CSP policy, it can block all kinds of things, like eval(..): https://developer.mozilla.org/en-US/docs/Security/CSP

@ecofi
Copy link
Author

ecofi commented Mar 13, 2014

I think that I missed a piece of information. The error appears when I try to use acorn.js as part of a Chrome extension. JS running in chrome extensions have higher privilege compared to JS embedded in "normal" web page. A solution would be to allow the CSP policy. However, this is not really my intention. Furthermore, it seems in that in chrome apps it is not possible to switch off this CSP policy (https://developer.chrome.com/apps/contentSecurityPolicy). Nevertheless, it makes me think that such parsing may not work for my scenario. I guess that I have to rethink my approach.

@marijnh
Copy link
Member

marijnh commented Mar 13, 2014

Okay, that is nasty. Both Acorn and Tern use new Function for some simple syntactic abstraction, assuming that since it is part of the language, it is a safe feature to use.

But having the libraries blocked in Chrome apps is sort of crappy. I think this is an awful (though somewhat understandable) decision on the part of the Chrome team, but I don't expect they'll revert it. For Acorn, we could feature-detect the problem and fall back to a regexp. For Tern, which generates simple record constructors, I'm not quite sure what to fall back to.

@marijnh
Copy link
Member

marijnh commented Mar 13, 2014

Does the first answer to http://stackoverflow.com/questions/11897112/eval-in-chrome-package-app show a way around this?

@marijnh
Copy link
Member

marijnh commented Mar 25, 2014

@ecofi Did you try the solution I linked? Did it help?

@mozfreddyb
Copy link

Using 'unsafe-eval' helped in our case.

I'm not sure how feature detection could work in this scenario, but willing to look into it.
@marijnh would you take a patch that avoids eval and Function when they don't work?

@marijnh
Copy link
Member

marijnh commented Apr 9, 2014

@mozfreddyb Good to hear that there is a workaround. In acorn, you can fall back on a regexp when eval isn't available. In Tern, it'd be a bit more awkward, and you might want to rewrite the constraint helper entirely, though I expect the result will be quite a bit more clunky. Why do you want to fix this, when there is a workaround?

@mozfreddyb
Copy link

The most common use case of Content Security Policy (CSP) is to provide defense in depth and mitigate Cross-Site Scripting (XSS) issues. This can be done with a strong policy that disallows scripts and event handlers inline in HTML and forbids eval, Function and setTimeout and setInterval (with string parameters).

The latter is needed to make sure that user supplied content cannot lead to arbitrary script execution (i.e. so-called DOM XSS). The workaround of supplying 'unsafe-eval' to the policy basically means reducing the security benefits of CSP and re-opens the door to DOM XSS.

Avoiding eval in favor of regexes in acorn would help us maintain a higher level of security :)

@marijnh
Copy link
Member

marijnh commented Apr 9, 2014

If the error raised by this security policy can be caught, feature-detecting it is trival. If not, see if the current policy can be accessed from scripts somehow, and failing that, maybe set up some mechanism for being able to configure Acorn to not use new Function.

@getify
Copy link

getify commented Apr 9, 2014

If the error raised by this security policy can be caught

By design (IIUC), the CSP security errors are not catchable. They both report to the console and can optionally do a ping to notify a remote logging service.

see if the current policy can be accessed from scripts somehow

Again, IIUC, by design this is impossible.

set up some mechanism for being able to configure Acorn

IMO this is the most probable path of success.

@mozfreddyb
Copy link

Calling eval will throw a (very generic) Error that can be caught using normal try/catch blocks. Sadly, the error message differs widely from browser to browser, so you'd have to make sure the detection stay up to date with browser error messages.

@marijnh
Copy link
Member

marijnh commented Apr 9, 2014

so you'd have to make sure the detection stay up to date with browser error messages.

No, you don't. If try { new Function("return 1")(); } catch(e) { ... } enters the catch block, new Function is disabled. There doesn't seem to be a realistic other scenario where an exception would be raised in that block.

@getify
Copy link

getify commented Apr 9, 2014

Just tried it. It is "catchable" in that it fires the try/catch, but it's not "catchable" in the sense that it doesn't prevent it from being reported to the console, nor does it prevent the remote beaconing of errors.

As such, I don't think this would be an effective "feature test" in that we can't prevent the side-effects.

@mozfreddyb
Copy link

So what's the take away for acorn? Be noisy and do try/catch or be safer and always use regexes?

@getify
Copy link

getify commented Apr 11, 2014

So what's the take away for acorn?

As was said above, IMO this:

maybe set up some mechanism for being able to configure Acorn

@Sevin777
Copy link

Sevin777 commented Jun 1, 2014

I am writing a Chrome App (not extension) that uses CodeMirror and Tern/Acorn. I just ran into the same issue, it seems like its possible to use a work around using a sandbox https://github.com/GoogleChrome/chrome-app-samples/tree/master/sandbox, but it looks like it would be quite difficult to implement as the main apps javascript can only communicate with the sandboxed javacsript via a crude messaging system. Unfortunately this is my qutting point for writing this app as it was a side project that I can't devote a lot of time to :(. It would be excellent if Tern/Acorn could include the feature detection and fallback (or if you could provide an example of it). However, from what I just read in this thread it sounds like it might not even be possible for Tern to work without using eval/new function?

UPDATE-- I got it to work! I simply implemented a 'fake' worker that calls the sandbox code. This is how:

  • Create a sandbox (see link above for demo) that contains tern and all of its dependencies (acorn)
  • Start tern with the option to use web worker
  • modify the WorkerServer for tern to use a fake worker that posts messages to the sandbox:
function WorkerServer(ts) {
    //fake web worker that communicates with sandbox instead of web worker
    function fakeWorker() {
        var self = this;
        //post message (just like web worker)
        this.postMessage = function (message) {
            document.getElementById('sandboxFrame').contentWindow.postMessage(message, '*');
        }
        //message received
        this.onmessage = null;
        //error .. not sure how this should be implemented
        this.error = null;        
        //bind listener for message returned from sandbox
        window.addEventListener('message', function (event) {            
            if (typeof (self.onmessage === 'function')) {              
                self.onmessage(event);
            }
        });        
    }
    var worker = new fakeWorker();//var worker = new Worker(ts.options.workerScript); //- use fake worker
//rest of code is the same.....




- Modify the worker.js file as follows (include the file in the sandbox):

var secureEventSource = null;
var secureEventOrigin = null;
//add event listener to imitate web worker receiving message
window.addEventListener('message', function (e) {    
    secureEventSource = e.source;
    secureEventOrigin = e.origin;
    onmessage(e);//call fake worker on message event  
});
//call this instead of postMessage- this calls secure app (repalces webworkers postmessage code)
function postMessageToSecure(e) {    
    secureEventSource.postMessage(e, secureEventOrigin);
}
//message received
function onmessage(e) {    
    var data = e.data;    
    switch (data.type) {
        case "init": return startServer(data.defs, data.plugins, data.scripts);
        case "add": return server.addFile(data.name, data.text);
        case "del": return server.delFile(data.name);
        case "req": return server.request(data.body, function (err, reqData) {
            postMessageToSecure({ id: data.id, body: reqData, err: err && String(err) });
        });
        case "getFile":
            var c = pending[data.id];
            delete pending[data.id];
            return c(data.err, data.text);
        default: throw new Error("Unknown message type: " + data.type);
    }
};
var server;
var nextId = 0, pending = {};
function getFile(file, c) {    
    postMessageToSecure({ type: "getFile", name: file, id: ++nextId });
    pending[nextId] = c;
}

function startServer(defs, plugins, scripts) { 
    if (scripts) importScripts.apply(null, scripts);
    server = new tern.Server({
        getFile: getFile,
        async: true,
        defs: defs,
        plugins: plugins
    });
}
  • The CodeMirror Tern plugin (Tern.js) code must be included in both the sandbox and the secure source
  • The Tern.js and Acorn.js files that contain eval and new function() are only included in the sandbox

@gfwilliams
Copy link

I've just bumped into this with a packaged Chrome Web App (the Web IDE for the Espruino JS microcontroller)... I tried the CSP entry in the manifest, and I get the following:

There were warnings when trying to install this extension:
'content_security_policy' is only allowed for extensions and legacy packaged apps, but this is a packaged app.

So it sounds like as far as CSP goes, I'm out of luck. @sevin7676's solution looks really promising though. Is there any complete example of a working version of CodeMirror that's using this? Is it https://github.com/sevin7676/Chrome-Webpad?

@Sevin777
Copy link

Yes, Chrome-Webpad has it working (the code is really messy because I was really just using it for a test)

@RReverser
Copy link
Member

Hi there. Until better solution is found, I've created prebuilder that builds acorn.js into CSP-safe version by precompiling and inlining all the predicate functions: https://github.com/RReverser/acorn-csp.

Resulting file is bigger, but doesn't contain new Function anymore so CSP error is not thrown (while speed benefit is saved).

@gfwilliams
Copy link

This is awesome - thanks!

@marijnh
Copy link
Member

marijnh commented Sep 8, 2014

@RReverser I would also be okay with including such a script in the bin/ dir of the main distribution, if you want to submit your code as a PR.

@RReverser
Copy link
Member

@marijnh Can do if you don't mind having esprima and recast in devDependencies.

@RReverser
Copy link
Member

@marijnh Actually I think after recent code simplifications esprima can be now easily replaced with acorn itself so we don't have to explicitly depend on it, but recast depends on espima-fb inside anyway.

@marijnh
Copy link
Member

marijnh commented Sep 12, 2014

I've pushed 91911bc and 0322beb to address this. You now get an acorn_csp.js file if you install from npm, which has the generated predicates pre-generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants