1318911 - Chrome code execution through several small bugs and user interaction

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P1)

Description

[Abdulrahman Alqabandi]

Attachment: DnD-PoC.html

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36

Steps to reproduce:

Open PoC and follow steps using a windows machine (if you cant run this on windows change the chrome code that executes calc into whatever else and then base64 it and replace the already existent base64.


This works on latest nightly, only other version tested is latest stable and it doesnt work.

The main bug here is the odd behavior where javascript: URLs coming from bookmarks (or in this case, the home button) will turn into chrome privileged windows after a reload occurs.

Granted, this is heavy on the user interaction but at the end of the day chrome code is being executed.


Actual results:

After a few steps, calc.exe is executed.


Expected results:

A lot of things. There is a good chance this will need to be broken up to multiple bugs but I'll wait your assessment first.

Comment 1

[Abdulrahman Alqabandi]

Note: I realize this is similar to asking a user to open up 'about:newtab' and type things up into the console. However, in this case there is nothing that indicates the user is within a privileged page, which is a tad more dangerous.

Comment 2

[Abdulrahman Alqabandi]

Here is another bug which allows us to gain a reference to a chrome window:

1. We need to either create a bookmark or set homepage to a javascript URL.
2. We then need to click the bookmark or the homepage.
3. Using the javascript url PoC below, we should gain a reference to a chrome window through window.opener

javascript:open('data:text/html,window.opener should be a chrome window');`<script>location.reload()</script>`

Comment 3

[:Gijs (he/him)]

The dnd to the home button only works in e10s, because of bug 1317023. It shouldn't work.

Then there's the fact that the web console works on chrome pages, and that it doesn't warn you. It should be using the same paste warnings / blockings that it does on normal webpages, if not more severe ones. In fact, it probably shouldn't be working at all if devtools.chrome.enabled isn't set. I'll file a bug for that in a second.

Then the thing that I find very surprising is that the result of the javascript: URL loads a document without system principal, but then that document calls location.reload() and the about:blank doc that loads *is* using the system principal? This feels like another way to (ab)use bug 1284395. It doesn't seem like the fix we used there is sufficient. Boris, can we categorically forbid doing a load that inherits principals on an about:blank page (that doesn't have a "normal" website codebase principal because it was opened by some website) in the way that pre-bug 1284395 code did for 'open link in new tab' and that the homepage code still does? I don't think the web is able to cause such a thing to happen without user interaction, and I can't think of a reasonable reason to want it for chrome code. Playing whack-a-mole in frontend code to avoid this happening when user interaction gets involved feels like a losing battle. :-\

Comment 4

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi[test] from comment #2)
> Here is another bug which allows us to gain a reference to a chrome window:
> 
> 1. We need to either create a bookmark or set homepage to a javascript URL.
> 2. We then need to click the bookmark or the homepage.
> 3. Using the javascript url PoC below, we should gain a reference to a
> chrome window through window.opener
> 
> javascript:open('data:text/html,window.opener should be a chrome
> window');`<script>location.reload()</script>`

... but compartments and wrappers should mean that you can't actually do (almost - besides calling window.close() and maybe postMessage or something) anything with that window, right?

Comment 5

[:Gijs (he/him)]

FWIW, it seems like normal users (on non-prerelease channels) will at least not be allowed to drop stuff in the web console without getting a "scam" warning. Those are disabled on nightly and devedition, and they get disabled if you use the console, one or more of which probably applies to everybody on this bug. :-)

Comment 6

[Abdulrahman Alqabandi]

Attachment: HomeDnD

> ... but compartments and wrappers should mean that you can't actually do
> (almost - besides calling window.close() and maybe postMessage or something)
> anything with that window, right?

Yes the chrome window object we end up with is protected, as in we cant execute anything. But I understand that gaining a reference to a chrome window (other than manually having a user go to 'about:newtab' after we open a window using open()) is a security problem, right? 

As you pointed out in comment 5, the drag and drop is blocked the same way as pasting (did not expect that) so I guess the part where you DnD the final payload would never happen had this landed on stable. But, at the very least there is some sort of bug here.

> The dnd to the home button only works in e10s, because of bug 1317023. It shouldn't work.

It works for me with e10s turned off and on. I attached a PoC for that part only, could you try again? (I tested latest nightly 53.0a1 (2016-11-21) (64-bit))

Comment 7

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi[test] from comment #6)
> > The dnd to the home button only works in e10s, because of bug 1317023. It shouldn't work.
> 
> It works for me with e10s turned off and on. I attached a PoC for that part
> only, could you try again? (I tested latest nightly 53.0a1 (2016-11-21)
> (64-bit))

Security Error: Content at https://bug1318911.bmoattachments.org/attachment.cgi?id=8812525&t=85Owg4FJiCEMvr7ivONssT may not load or link to about:Check-The-Other-Tab!|javascript:atob('snip').

is what happens on the original testcase. The new poc doesn't get blocked. Looks like we should improve those checks as well.

Comment 8

[Abdulrahman Alqabandi]

Oh ok, my bad, the original PoC does indeed rely on Bug 1317023 for that part.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> but then that document calls location.reload() and the about:blank doc that
> loads *is* using the system principal?

I believe bug 1297338 introduced a regression that could cause this to happen, but we fixed it in bug 1308889.... and that was fixed a while ago now.

Ah, I see.  Bug 1297338 introduced _another_ regression too.  In nsDocShell::LoadHistoryEntry in the javascript: case we do:

12475       rv = CreateAboutBlankContentViewer(triggeringPrincipal, nullptr,
12476                                          aEntry != mOSHE);

that's wrong.  It should be passing the principalToInherit, not the triggeringPrincipal.  If I do that, the testcase from comment 2 starts opening one window after another and there is no chrome anything afaict.

I'll post a patch; not sure how to write a sane test for this.

> Boris, can we categorically forbid doing a load that inherits principals on an about:blank page

What makes about:blank special for that sort of check?

Comment 10

[Boris Zbarsky [:bzbarsky]]

ccing Christoph and Tanvi; see comment 9.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: When creating an about:blank content viewer during a history navigation to a javascript: URI, use the right principal

Comment 12

[:Gijs (he/him)]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #9)
> > Boris, can we categorically forbid doing a load that inherits principals on an about:blank page
> 
> What makes about:blank special for that sort of check?

Fair point. I suppose it all boils down to killing inheriting chrome principals full stop - but as we've discussed before, that's nontrivial ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=98d1af6eb9410ab8cd9afaae01e09cb24ca6e08f&selectedJob=29041543 ; bug 1185015 ).

Comment 13

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi[test] from comment #6)
> Created attachment 8812827 [details]
> HomeDnD
> 
> > ... but compartments and wrappers should mean that you can't actually do
> > (almost - besides calling window.close() and maybe postMessage or something)
> > anything with that window, right?
> 
> Yes the chrome window object we end up with is protected, as in we cant
> execute anything. But I understand that gaining a reference to a chrome
> window (other than manually having a user go to 'about:newtab' after we open
> a window using open()) is a security problem, right? 

We treat it as a defense in depth problem, yes. I believe the patch bz has attached to this bug will fix this case as well.

(In reply to Abdulrahman Alqabandi[test] from comment #8)
> Oh ok, my bad, the original PoC does indeed rely on Bug 1317023 for that
> part.

Right, but it's not essential - as your other poc points out, using http://localhost/ (or some other URI that displays a page suggesting to open the other tab) would work just as well. We should update the homepage checks to check all linked URIs and disable inheritance for them. I believe dnd of a |-separated set of URLs doesn't let you create multiple bookmarks or something like that - it'd use the | as part of the initial link.

Comment 14

[Abdulrahman Alqabandi]

Attachment: PoCV2.html

Here is a much better PoC that uses a different approach into social engineering a potential victim into executing malicious code inside the chrome window. No 'allow pasting' window appears even on stable.

Comment 15

[Abdulrahman Alqabandi]

Note: forgot one thing, please modify the 'localhost/q.js' within the PoC into your own hosted .js that has the following:
alert(Components.utils);

Comment 16

[Daniel Veditz [:dveditz]]

Based on the regressing bug I assume Beta (Firefox 51) isn't affected, but that the current Dev Edition (Firefox 52) is.

capping security severity to sec-moderate based on the suspicion-raising user interaction

Comment 17

[Tanvi Vyas[:tanvi]]

Comment on attachment 8812849 [details] [diff] [review]
When creating an about:blank content viewer during a history navigation to a javascript: URI, use the right principal

># HG changeset patch
># User Boris Zbarsky <[e-mail]>
># Date 1479752883 18000
>#      Mon Nov 21 13:28:03 2016 -0500
># Node ID f20b3335fe6f0e563985727b6b5d5875b19ed82b
># Parent  04cf2044c072a838eed9b58a966f6ba7b91ab670
>Bug 1318911.  When creating an about:blank content viewer during a history navigation to a javascript: URI, use the right principal.  r=tanvi
>
>diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
>--- a/docshell/base/nsDocShell.cpp
>+++ b/docshell/base/nsDocShell.cpp
>@@ -12467,17 +12467,17 @@ nsDocShell::LoadHistoryEntry(nsISHEntry*
>   if (NS_FAILED(rv) || isJS) {
>     // We're loading a URL that will execute script from inside asyncOpen.
>     // Replace the current document with about:blank now to prevent
>     // anything from the current document from leaking into any JavaScript
>     // code in the URL.
Comment explicitly states to not use the current document, which we may be doing by using triggeringPrincipal (since it often equals loadingPrincipal).

What will the principalToInherit be in this case?  Is it different depending on the scenario, or always a null principal?

>     // Don't cache the presentation if we're going to just reload the
>     // current entry. Caching would lead to trying to save the different
>     // content viewers in the same nsISHEntry object.
>-    rv = CreateAboutBlankContentViewer(triggeringPrincipal, nullptr,
>+    rv = CreateAboutBlankContentViewer(principalToInherit, nullptr,
>                                        aEntry != mOSHE);
> 
>     if (NS_FAILED(rv)) {
>       // The creation of the intermittent about:blank content
>       // viewer failed for some reason (potentially because the
>       // user prevented it). Interrupt the history load.
>       return NS_OK;
>     }

Comment 18

[Boris Zbarsky [:bzbarsky]]

> Comment explicitly states to not use the current document

Right.  The current document is generally totally unrelated to both triggeringPrincipal and principalToInherit in this case, because those come from the history entry being loaded, while the current document is the thing loaded right now.  They have nothing to do with each other.

> What will the principalToInherit be in this case?

Whatever the principalToInherit was when we did the load that caused the history entry to be created.  It doesn't have to be a nullprincipal, but it _can_ be nullptr if the history entry was not created by a load that inherited principals.  If it's nullptr, CreateAboutBlankContentViewer will end up creating an about:blank document whose principal is a codebase principal for about:blank.

Comment 19

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi[test] from comment #14)
> Created attachment 8812909 [details]
> PoCV2.html
> 
> Here is a much better PoC that uses a different approach into social
> engineering a potential victim into executing malicious code inside the
> chrome window. No 'allow pasting' window appears even on stable.

Filed bug 1319381, bug 1319382 on (a) applying the 'paste' warning to the GCLI, and (b) not letting it inject arbitrary crap (and maybe do other stuff) into chrome scopes.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Tanvi, does comment 18 answer your questions?

Comment 21

[Tanvi Vyas[:tanvi]]

Comment on attachment 8812849 [details] [diff] [review]
When creating an about:blank content viewer during a history navigation to a javascript: URI, use the right principal

Please push to try if you haven't already.

Comment 22

[Boris Zbarsky [:bzbarsky]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c997f57f2d57c40cc7d3a362fab426da0c514277&selectedJob=32294255

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8812849 [details] [diff] [review]
When creating an about:blank content viewer during a history navigation to a javascript: URI, use the right principal

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1297338.
[User impact if declined]: Security issue.
[Is this code covered by automated tests?]:  No.  I haven't thought of a good
   way to add tests for this yet.
[Has the fix been verified in Nightly?]:  Not yet.
[Needs manual test from QE? If yes, steps to reproduce]: See comment 0.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: I don't think it is.
[Why is the change risky/not risky?]: This is restoring something closer to
  the behavior we have in 51.
[String changes made/needed]: None.

Comment 24

[Carsten Book [:Tomcat]]

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

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8812849 [details] [diff] [review]
When creating an about:blank content viewer during a history navigation to a javascript: URI, use the right principal

Fix for regression from 52, let's uplift it and verify in aurora.

Comment 26

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9a2e6dec9468

Comment 27

[Abdulrahman Alqabandi]

(In reply to :Gijs from comment #19)
> Filed bug 1319381, bug 1319382 on (a) applying the 'paste' warning to the
> GCLI, and (b) not letting it inject arbitrary **** (and maybe do other
> stuff) into chrome scopes.

May I get access/cc to those bugs?

Comment 28

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi[test] from comment #27)
> (In reply to :Gijs from comment #19)
> > Filed bug 1319381, bug 1319382 on (a) applying the 'paste' warning to the
> > GCLI, and (b) not letting it inject arbitrary **** (and maybe do other
> > stuff) into chrome scopes.
> 
> May I get access/cc to those bugs?

Done.

Comment 29

[Petruta Horea [:phorea], Desktop QA]

I used https://bugzilla.mozilla.org/attachment.cgi?id=8812525 test case with Aurora 52.0a2 2016-11-21 and 53.0a2 2017-03-01 builds under Win 10 64-bit and I see no differences between them after following the steps.

After executing the console code, calc.exe is not opened in any version, although I would expect it to be opened with 52.0a2 Aurora. Am I missing something here?

Is this fixed for you in latest Firefox 52 and Aurora 53?

Thank you!

Comment 30

[Abdulrahman Alqabandi]

(In reply to Petruta Rasa [QA] [:petruta] from comment #29)
> I used https://bugzilla.mozilla.org/attachment.cgi?id=8812525 test case with
> Aurora 52.0a2 2016-11-21 and 53.0a2 2017-03-01 builds under Win 10 64-bit
> and I see no differences between them after following the steps.
> 
> After executing the console code, calc.exe is not opened in any version,
> although I would expect it to be opened with 52.0a2 Aurora. Am I missing
> something here?
> 
> Is this fixed for you in latest Firefox 52 and Aurora 53?
> 
> Thank you!

I can confirm it does not work on FireFox 53.0a2 (2017-03-02) (64-bit) and does not work on FireFox 52.0 (32-bit)

Also, I think I mistakenly reported it 52 as being affected, but really I only tested latest nightly and stable. It worked on nightly, not stable. I never saw it work on any other version.

Comment 31

[Petruta Horea [:phorea], Desktop QA]

Hi Boris,

Was this only partially fixed? (please see above comments). If so, could you please indicate how to verify that part? Thank you!

Comment 32

[Boris Zbarsky [:bzbarsky]]

It's not clear what "it" is in comment 30.  But I assume it means "the PoC".  As in comment 30 says the bug is not present on current tip or on 52 (presumably current 52?).  As far as I know this is completely fixed, but please tell me if I'm missing something!

I can't speak to the calc.exe testcase, because I could never run it anyway (not on Windows).  I used the testcase from comment 2 for testing: save that URL as a bookmark, click the bookmark, examine window.opener.

Comment 33

[Petruta Horea [:phorea], Desktop QA]

Attachment: Results on old and newer Aurora with STR from comment 2 and window.opener

I assumed comment 30 said the issue is still present.

Here's my results using the testcase from comment 2 and window.opener on 2016-11-21 52.0a2 and 2017-03-01 53.0a2:
- 52.0a2: nothing happens on the first tab; window.opener shows Object -> about:blank

- 53.0a2: the first tab is continuously reloaded; window.opener shows Window -> about:blank (OR less frequently Window → javascript:open('data:text/html,window.opener%20should%20be%20a%20chrome%20window');`<script>location.reload()</script>`)

Please advise.

Comment 34

[Boris Zbarsky [:bzbarsky]]

> - 52.0a2: nothing happens on the first tab; window.opener shows Object -> about:blank

Yes, that is showing a bug: the "Object" instead of "Window" is because it's an opaque wrapper for a chrome window.  That should not happen for web content.

> - 53.0a2: the first tab is continuously reloaded

Yes, that's expected given the testcase, since it _does_ call location.reload().

> window.opener shows Window ->

And that shows that we no longer have a chrome window opener.

Which verifies the bits _I_ verified before checking in the patch.

Whether any of this fixes the original bug with the code execution is something that still needs to be verified, of course....

Comment 35

[Petruta Horea [:phorea], Desktop QA]

Thank you!

I managed to get calc.exe opened on a Win 8.1 32-bit machine using Nightly 53.0a1 2016-11-19 and the first PoC attached.
The same testcase with latest Aurora 53.0a2 and Firefox 52.0 doesn't execute calc.exe anymore.

I can now confirm the fix for that testcase and the one from comment 2 (thanks for clarifying there too).

Comment 36

[Petruta Horea [:phorea], Desktop QA]

Marking as verified based on above comments.