1281787 - foo.com can access view-source:blob:http://foo.com/<uuid> for valid blob URIs (but not view-source:http://foo.com/ )

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[:Gijs (he/him)]

So here I was trying to write a good test for bug 1277583... I think I tried a little too hard. I don't know to what degree this is exploitable - I haven't tried - I just find it very surprising, and it seems to violate my(/our?) ideas about what DANGEROUS_TO_LOAD restrictions on schemes do and don't prevent.

STR:

1. open http://www.mozilla.org/
2. open the web console
3. run:

w = window.open("view-source:http://www.mozilla.org/", "_blank");

(this will fail, which is correct and expected.)

4. run:

u = URL.createObjectURL(new Blob(["this is a blob"]));
w = window.open("view-source:" + u, "_blank");

this opens a new tab (or window) with the view-source URI, and the content window has access to the result of "w", can read innerHTML etc.

AFAICT this won't work if the inner blob URI is invalid or belongs to a different domain. Small mercies. But even so, I don't think it should work at all. I'll leave nested view-source:blob stuff out of the test in bug 1277583 for now.

Comment 1

[Daniel Veditz [:dveditz]]

I assume you've disabled the popup blocker, or disabled it for www.mozilla.org -- I couldn't run your STR without doing that.

---(1)---
If I open http://www.mozilla.org/ I get redirected to http_S_://www.mozilla.org/

window.open("view-source:http://www.mozilla.org") is therefore cross-origin and gives me
  Error: Access to 'view-source:http://www.mozilla.org/' from script denied

However, same-origin window.open("view-source:https://www.mozilla.org/") works.


---(2)---
if I add a link to the page, "view-source:"+same-origin URL will open (e.g. "view-source:https://www.mozilla.org"). A cross-origin view-source link will fail.


-------

We've got a bug here but it's not related to "blob:"

Comment 2

[:Gijs (he/him)]

(In reply to Daniel Veditz [:dveditz] from comment #1)
> I assume you've disabled the popup blocker, or disabled it for
> www.mozilla.org -- I couldn't run your STR without doing that.

Oh, um, yes, probably as a side-effect of testing some other bug, yes. Sorry!


> ---(1)---
> If I open http://www.mozilla.org/ I get redirected to
> http_S_://www.mozilla.org/
> 
> window.open("view-source:http://www.mozilla.org") is therefore cross-origin
> and gives me
>   Error: Access to 'view-source:http://www.mozilla.org/' from script denied
> 
> However, same-origin window.open("view-source:https://www.mozilla.org/")
> works.
> 
> 
> ---(2)---
> if I add a link to the page, "view-source:"+same-origin URL will open (e.g.
> "view-source:https://www.mozilla.org"). A cross-origin view-source link will
> fail.
> 
> 
> -------
> 
> We've got a bug here but it's not related to "blob:"

This is surprising to me. On https, and with view-source:https://..., I still see:

Error: Access to 'view-source:https://www.mozilla.org/' from script denied

using:
window.open("view-source:" + location.href, "_blank");

(but the same happens without "_blank".)

What build are you testing on? I checked both 48 beta and nightly. Both produce this error. I just checked a clean profile on Nightly, and that shows this error and no popup blocker warning for the JS above, but shows the popup blocker when I try with a blob: URI.

In fact, one thing that might explain this: is security.view-source.reachable-from-inner-protocol perhaps set to true in the build/profile you're using to test with?

Comment 3

[Daniel Veditz [:dveditz]]

D'oh! yes, I had set that pref at some point.

Comment 4

[Jay Gilbert]

This is incredibly interesting to me, is view-source: still privileged in some way or does it trigger chrome level XBL bindings?  I'm not going to NI anyone and test this myself for now.

IIRC I've been able to trigger view-source: loads that were same origin recently.  I'll run back through my standard set of things I test to see if that's correct.

Very interesting indeed.

Comment 5

[Jay Gilbert]

(In reply to :Gijs Kruitbosch from comment #0)
> u = URL.createObjectURL(new Blob(["this is a blob"]));
> w = window.open("view-source:" + u, "_blank");
> 
> this opens a new tab (or window) with the view-source URI, and the content
> window has access to the result of "w", can read innerHTML etc.

This gets even more interesting if you specify the type for the blob as 'application/x-view-source'.  You will get an exception generated at the content level with '<no message>' for the message.

If you check constructor.name for this exception you will get "Object" yet this constructor is not the Object function of that content window, or any window that is a child of it(testing this in a child iframe).

I'll dig into this deeper tomorrow but that has me intrigued.  I believe it's an exception that shouldn't be accessible to web content.  My theory right now is that adding 'view-source:' to a blob URL that has the type 'application/x-view-source' is attempting to feed it through the view-source: inner workings twice and bailing out with no clue.

Comment 6

[Jay Gilbert]

It appears that the constructor is the Object function from the window that *should* have loaded if it didn't bail out.  Does anyone have any what is going on with this?  

I know everyone is taking time off etc, so I'm just going to leave this here.

Comment 7

[Jay Gilbert]

(In reply to Jay Gilbert from comment #5)
> I'll dig into this deeper tomorrow but that has me intrigued.  I believe
> it's an exception that shouldn't be accessible to web content.  My theory
> right now is that adding 'view-source:' to a blob URL that has the type
> 'application/x-view-source' is attempting to feed it through the
> view-source: inner workings twice and bailing out with no clue.

I'm wrong here, this is actually because I'm testing in a child iframe.  

(In reply to Jay Gilbert from comment #6)
> It appears that the constructor is the Object function from the window that
> *should* have loaded if it didn't bail out.  Does anyone have any what is
> going on with this?  

With what I see so far I'm wrong here as well.  I think we need some more eyes on this one.  I do have some chrome level errors being generated here in regards to session storage.

Comment 8

[:Gijs (he/him)]

(In reply to Jay Gilbert from comment #7)
> (In reply to Jay Gilbert from comment #5)
> > I'll dig into this deeper tomorrow but that has me intrigued.  I believe
> > it's an exception that shouldn't be accessible to web content.  My theory
> > right now is that adding 'view-source:' to a blob URL that has the type
> > 'application/x-view-source' is attempting to feed it through the
> > view-source: inner workings twice and bailing out with no clue.
> 
> I'm wrong here, this is actually because I'm testing in a child iframe.  

Iframes are still special for view-source: bug 1243791.

Comment 9

[Daniel Veditz [:dveditz]]

sec-low because it's same-origin and the page could read the data directly anyway?

Comment 10

[:Gijs (he/him)]

Attachment: Tentative patch

Kyle, how about this? It passes dom/base/test/test_mozfiledataurl.html so I'm assuming it doesn't break the web.

I initially thought we could just move the subsumes check down below the other protocol flag checks, but I think that would allow blob URIs from foo.com to link to blob URIs from bar.com, because of http://searchfox.org/mozilla-central/source/caps/nsScriptSecurityManager.cpp#823-827

So I ended up doing some refactoring... does this look sane to you? (I'll take a less confusing name for CheckLoadURIFlags, I just couldn't think of one...)

Comment 11

[:Gijs (he/him)]

Comment on attachment 8767863 [details] [diff] [review]
Tentative patch

Seems Kyle is away. Boris, do you have time to look at this?

Comment 12

[Boris Zbarsky [:bzbarsky]]

I'd like to understand the actual threat model here.  That is, what are we trying to fix?

Looking at bug 1172165, the "read the cache" issues don't see very relevant here, right?  Are we just trying to fence around the bugs being cited in "has caused quite a few security bugs in Firefox"?

I guess maybe that last is worth doing...

Comment 13

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8767863 [details] [diff] [review]
Tentative patch

Is there a reason not to pass the sourceBaseURI/targetBaseURI to CheckLoadURIFlags, instead of regetting them from the sourceURI/targetURI?

>+    bool hasFlags = false;

Please move this down to right before the first use.

>+    CheckLoadURIFlags(nsIURI* aSourceURI, nsIURI* aTargetURI, uint32_t aFlags);

Needs documentation.

r=me with those.

Comment 14

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] from comment #12)
> Are we just trying to fence around the bugs being cited in
> "has caused quite a few security bugs in Firefox"?
> 
> I guess maybe that last is worth doing...

Defense in depth / shut all the things that look like holes (even if they're dead ends right now) is pretty much my motivation, yeah. view-source:blob from same-origin blobs is not so bad, I just think there ought not to be exceptions. DANGEROUS_TO_LOAD needs to actually mean that, never mind what's in the inner URI chain.

Plus concretely that I want to land the test in 1277583 and was told not to do that until the blob thing got sorted out (which maybe I should (have) push(ed) back harder on...).

Comment 15

[Jay Gilbert]

(In reply to :Gijs Kruitbosch from comment #14)
> (In reply to Boris Zbarsky [:bz] from comment #12)
> > Are we just trying to fence around the bugs being cited in
> > "has caused quite a few security bugs in Firefox"?
> > 
> > I guess maybe that last is worth doing...
> 
> Defense in depth / shut all the things that look like holes (even if they're
> dead ends right now) is pretty much my motivation, yeah. view-source:blob
> from same-origin blobs is not so bad, I just think there ought not to be
> exceptions. DANGEROUS_TO_LOAD needs to actually mean that, never mind what's
> in the inner URI chain.
> 
> Plus concretely that I want to land the test in 1277583 and was told not to
> do that until the blob thing got sorted out (which maybe I should (have)
> push(ed) back harder on...).

I'll be trying to step up my game and help you out with this as much as possible.  I'll get up with you on IRC so we can discuss things in depth.  

I know I was asked to do similar, but honestly I'm always afraid I'll muck things up more than actually help.  I'll be in touch, and probably have some new leads I;m working on to discuss with you.

Comment 16

[:Gijs (he/him)]

Attachment: Patch v2

I addressed the comments. Would have just landed but I noticed when testing this against an updated version of the tests in 1277583 that blob URIs were now erroring to the console. I fixed that in this version, but I'd appreciate if you can doublecheck that that's OK / makes sense.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8770503 [details] [diff] [review]
Patch v2

r=me

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fb5fa9e3a52b