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

UP-01-003 Worker-based Isolation does not prevent XHR & Sockets etc. #365

Open
cure53 opened this issue Aug 31, 2014 · 9 comments
Open

UP-01-003 Worker-based Isolation does not prevent XHR & Sockets etc. #365

cure53 opened this issue Aug 31, 2014 · 9 comments
Labels

Comments

@cure53
Copy link

cure53 commented Aug 31, 2014

Note: This issue technically affects FreeDOM. Please move the issue if necessary.

Freedom.js uses Web workers to isolate freedom contexts from each other and prevent them from directly accessing the DOM, which seems to be intended as a security measure for isolating different modules. However, web workers are not designed to provide secure isolation of code; they are designed for running heavyweight tasks in the background, with no direct access to the DOM or the window resulting as a side effect.

Web workers can e.g. still use XMLHttpRequest to perform HTTP requests to their origin, which is the same as the origin of the website that started the worker, they can load scripts without muted errors, they can use IndexedDB and so on. In a typical web application, the ability to perform XHRs in the name of the application’s domain effectively gives the worker full access to the application. Similarly, in a chrome application, the worker is not able to use the chrome.* APIs, but it can still use XMLHttpRequest to talk to all origins that are whitelisted in the application manifest.

This issue could be mitigated by erasing all potentially harmful objects in the global scope of the worker, but that solution seems somewhat brittle. We recommend switching to frame-based isolation with sandboxing instead as frames are designed for security. The following list of objects should be considered for deactivation:

  • Worker
  • WebSocket
  • XMLHttpRequest
  • XMLHttpRequestUpload
  • importScripts

The code to deactivate the object should make use of ES5’s Object.defineProperty()/Object.defineProperties(). This is because of the browser specific behavior of language features such as delete. While simply deleting an object inside a worker suffices in Firefox, on Chrome and related browsers it has no effect. Our tests found the following code to be most efficient:

Object.defineProperties(this, {
    'XMLHttpRequest': {value: null, configurable: false},
    'XMLHttpRequestUpload': {value: null, configurable: false},
    'Worker': {value: null, configurable: false},
    'importScript': {value: null, configurable: false},
    'WebSocket': {value: null, configurable: false}
});

console.log(new XMLHttpRequest());
console.log(new XMLHttpRequestUpload());
console.log(new Worker());
console.log(importScript());
console.log(new WebSocket());
@cure53 cure53 added P2 labels Aug 31, 2014
@willscott
Copy link
Member

One issue with frames that makes us hesitant to use them, even with the sandbox attribute to limit communication is that the frame code often will run in the same event loop as the outer page, allowing for denial of service issues. Workers will definitely run in a separate event loop, which seems preferable.

@cure53
Copy link
Author

cure53 commented Aug 31, 2014

The attack surface for a sandbox Iframe is also significantly larger. There you have a full blown DOM, sometimes even visible to the user (contentEditable, resulting drag & drop attacks etc.). In the Worker, you can execute JavaScript - but no DOM calls. We should be able to fully mitigate XSS using sand-boxed Iframes. But DoS? Hard.

While we didn't manage to find a browser allowing to use a DOM inside a worker (e.g. by using XHR & XHR.responseType='document'), it does make sense to close that particular channel.

@cure53 cure53 changed the title UP-01-003 UP-01-003 Worker-based Isolation does not prevent XHR & Sockets etc. UP-01-003 Worker-based Isolation does not prevent XHR & Sockets etc. Sep 15, 2014
@iislucas
Copy link
Contributor

iislucas commented Oct 8, 2014

Freedom issue for this is here: freedomjs/freedom#110

@dborkan dborkan added this to the v2.0 Baxter release (After v1.0 Allardice launch) milestone Oct 20, 2014
@agallant agallant self-assigned this Oct 24, 2014
@agallant
Copy link
Collaborator

More detailed updates in freedom issue, but the short of it is this is the tentative whitelist:
https://docs.google.com/a/cs.washington.edu/spreadsheets/d/1kVAtIPgAeFBRtt7JbgTegT7nYg57JPl63Lt3S0Ac794/edit#gid=0

And there's a few commits that mask things not on the whitelist from the global scope of webworkers in freedom.

@dborkan
Copy link
Contributor

dborkan commented Oct 24, 2014

Can we clarify when we expect these changes to be in FreedomJS? We don't
believe fixing this issue should be blocking for the uProxy release to
Chrome/Firefox web stores (it is marked P2 and doesn't pose a threat to
uProxy users until we have untrusted freedom modules), and so we don't want
to have to do extra work to move XHRs to a core provider this quarter.
Will there be a way for some modules (e.g. freedom-social-facebook) to get
permission to use XHRs directly? Or can this be added to freedom 0.7?
Thanks

On Fri Oct 24 2014 at 12:29:09 PM soycode [email protected] wrote:

More detailed updates in freedom issue, but the short of it is this is the
tentative whitelist:

https://docs.google.com/a/cs.washington.edu/spreadsheets/d/1kVAtIPgAeFBRtt7JbgTegT7nYg57JPl63Lt3S0Ac794/edit#gid=0

And there's a few commits that mask things not on the whitelist from the
global scope of webworkers in freedom.


Reply to this email directly or view it on GitHub
#365 (comment).

@agallant
Copy link
Collaborator

I have less information/opinion about timeline, but re: modules that need access I think it'd be best to wrap it in an interface where the permissions can be specified in the manifest.

EDIT - which is basically the "move XHRs to a core provider", so yeah, if the decision is "not this quarter" then I can sit on this for the time being. Having the whitelist spreadsheet out there is still a good thing as people can peruse and comment as desired.

@iislucas
Copy link
Contributor

I'd suggest we focus on P1 issues this quarter, then we can all be excited about contributing stuff to the release. 🎆

But, yes, moving XHR to core provider is the right thing for freedom 0.7 / uproxy 2

@iislucas
Copy link
Contributor

Ah, and yes, whitelist spreadsheet is really useful!

@willscott
Copy link
Member

I think it's valuable to develop a definitive API of what is available in a module as part of the design document explaining the security properties of freedom.js so that we can go to outside security auditors and be able to say: this is what the freedom.js library claims to do for the system.

The response we've gotten so far from 2 different external parties was that they weren't comfortable auditing freedom because there was no clear sense of what properties they were trying to verify that it actually maintained.

It looks like the only thing that is pending removal that's being used at the moment is XHRs, which are used only in freedom-social-facebook. Presumably the fix-up work there would be part of the removal task, since it is so localized.

@gwpenn gwpenn removed this from the v-2.0 Baxter milestone Aug 2, 2016
@ghost ghost added P3 and removed P2 P1 C:Core labels Nov 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants