766607 - worker iframe content-type

Status: RESOLVED WONTFIX

(Firefox Graveyard :: SocialAPI, defect)

Description

[Shane Caraveo (:mixedpuppy)]

ensure iframe content-type for received data is appropriate for the frameworker.  only allow javascript content-types

Comment 1

[Shane Caraveo (:mixedpuppy)]

text/plain may be better

Comment 2

[Mark Hammond [:markh] [:mhammond]]

out of interest, why do we care?  By the time we can determine the content-type, the content will already be in the iframe (so any "damage" will have been done) - and there doesn't seem to be any damage that could be done anyway (ie, javascript is the "riskiest" content-type, but that is what we need)

Comment 3

[Shane Caraveo (:mixedpuppy)]

This came out of this mornings security review.  We don't want to have any chance of content actually having access to the full iframe.  right now you could get any type of content into the iframe.

we can catch this with nsIContentPolicy.shouldProcess.  I think there is also some way to modify the content type before loading, but I haven't verified that.

Another approach may be using XHR to retreive the js file, stick it in the sandbox, but we'd need to figure out how to get the right nsIPrincipal for both iframe and sandbox.

Comment 4

[Shane Caraveo (:mixedpuppy)]

I looked at nsIContentPolicy, it will watch ALL requests, probably not what we want.  I didn't finish looking into using nsIWebProgressListener as a way to block content.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Simon, Adam: Shane tells me you guys were part of the sec-review for this - I'm looking for clarification on the reasoning behind this (see also Mark's comment 2). 

The notes at https://wiki.mozilla.org/Security/Reviews/SocialAPI say "flash, java applets, etc running inside the window will not be bound by restrictions on what Javascript can do", but I don't quite understand that - the hidden iframe is type="content", so this should be no different than loading a page in a tab, so it's not clear what attack scenario would be mitigated by implementing this restriction.

Comment 6

[Adam Muntner [:adamm] (use NEEDINFO)]

Gavin/Mark:

The hidden iframe is a restricted sandbox, so from that perspective it's already 'different' in terms of being it's own security domain.

While you're right that that the same malicious code could be loaded into a tab, Simon and me were thinking along the lines that long-running Java and Flash become more likely in the sandbox than a tab - a tab is more easily/likely closed by a user, new page loaded in the tab, etc.

There are a few other points we discussed that led to our conclusion that Flash and Java etc applets should be blocked:

- Flash and Java applets are the two most targeted technologies by malware. 
- Allowing Java and Flash increases the attackable surface area, a lot. 
- Flash and Java applets have their own security model which doesn't always work how we would like. Once loaded from somewhere deemed safe by CSP, it follows it's own policy. For example, it can only read from place trusted by crossdomain.xml, but can write to anywhere on a network so long as it doesn't expect a response.
 
CSP in the browser has a way to block all of these kinds of applets, using an object-src source-list of 'none'

Is that something you can leverage from the chrome? Otherwise, we could include that as a security recommendation for developers who have pure Javascript code in order to better rotect their users.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Attachment: WIP patch that detects problems but doesn't actually act on them

Attaching a WIP patch - it does both contentType and SSL checking.  What is lacking is knowing what to do once we find a problem (bug 766616) and tests (which can wait until after bug 766616)

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 637014 [details] [diff] [review]
WIP patch that detects problems but doesn't actually act on them

>diff --git a/modules/SSLinfo.jsm b/modules/SSLinfo.jsm

>+// Indicates if an SSL channel is secure by returning NULL.
>+// A non-null result indicates the problem which might be suitable for logging

I don't really understand the purpose of this module - PSM already takes care of failing requests when the security state isn't suitable. You don't need to call verifyForUsage etc. yourself, so this module should be entirely unnecessary.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to Adam Muntner :adamm from comment #6)
> The hidden iframe is a restricted sandbox, so from that perspective it's
> already 'different' in terms of being it's own security domain.

I'm not sure what you mean by this. Note that the sandbox (where the code runs) and the iframe are two separate pieces.

> Is that something you can leverage from the chrome? Otherwise, we could
> include that as a security recommendation for developers who have pure
> Javascript code in order to better rotect their users.

We can easily disable loading of plugins in the iframe, if that's the concern (just toggle docShell.allowPlugins). That's a much more direct fix than trying to check content types. In fact, we may even be able to re-use code from bug 762993 - that implements a general purpose module that creates a hidden iframe with various security restrictions.

Comment 10

[Shane Caraveo (:mixedpuppy)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #9)
> (In reply to Adam Muntner :adamm from comment #6)

> We can easily disable loading of plugins in the iframe, if that's the
> concern (just toggle docShell.allowPlugins). That's a much more direct fix
> than trying to check content types. In fact, we may even be able to re-use
> code from bug 762993 - that implements a general purpose module that creates
> a hidden iframe with various security restrictions.

That creates an iframe one the top level window, we need ours on the hidden window.  The module could be modified to do that, or we could just set those flags on our sandbox.  I did try to do that, worked great on osx, accessing docshell on an iframe in the hiddenWindow on windows throws an exception.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

See my comment in that bug - we can easily have them change that.

How are you getting a reference to the docShell? The latest FrameWorker patch in bug 762569 doesn't seem to use createElementNS as suggested, which might explain you having problems.

Comment 12

[Adam Muntner [:adamm] (use NEEDINFO)]

Ga(In reply to :Gavin Sharp (use [e-mail] for email) from comment #9)
> (In reply to Adam Muntner :adamm from comment #6)
> > The hidden iframe is a restricted sandbox, so from that perspective it's
> > already 'different' in terms of being it's own security domain.
> 
> I'm not sure what you mean by this. Note that the sandbox (where the code
> runs) and the iframe are two separate pieces.

What I mean is that we have already set the expectation that this is a restricted environment. Minimizing functionality to that required for the business purpose is an important security design principle - expanding the attackable surface area with plugins that we can't really control well seems to violate that, especially given that Flash and Java applets are so prone to attack.

Comment 13

[Shane Caraveo (:mixedpuppy)]

bug 762569 now has a patch that turns off most available content features, we need a decision if that is enough or if we need to limit content types in the iframe.

Comment 14

[Adam Muntner [:adamm] (use NEEDINFO)]

Shane - can you explain the distinction? I'm not sure I understand.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

This bug as filed was about restricting loads in the iframe based on content type. The functionality we're implementing is that we're instead explicitly blacklisting certain features (like plugins, images, authentication dialogs, and window resizing). This mitigates the proposed attack scenarios, so I think it is sufficient.

Comment 16

[Adam Muntner [:adamm] (use NEEDINFO)]

Gotcha - that sounds good! Thank you.