546909 - (CVE-2010-0178) Firefox should not load chrome URLs dragged from plugins

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Paul Stone]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)

Firefox will load a chrome:// URL dragged from a Java applet onto the content area.

In a Java applet, the default 'mouse drag gesture recognizer' can be overridden so that a drag operation is initiated as soon as the mouse button is held down. As no mouse movement is required, a drag+drop can be 'forced' by getting the user to click the mouse button, and placing the drop target under the mouse cursor as soon as the drag operation has begun.

I have devised a test case that will allow chrome-privileged JS to be executed by getting the user to perform only three clicks. It works as follows:

1. Open two popup windows, one in front of the other (first click)
2. In the frontmost window, the user is persuaded to click on a Java applet, starting a drag of a chrome URL (any one will do)
3. Once the drag has started, the frontmost window is resized, so that the content area of the other popup window is underneath the mouse cursor.
4. When the drop is completed, the chrome URL loads in the background popup, and the foreground window is resized to it's original size.
5. The user performs another click on the foreground window. This time, a drag of a malicious Javascript URI is initiated.
6. The foreground window is shrunk again and the mouse button is released over the address bar of the background popup window.
7. The Javascript is executed with chrome privs.

I have tested this on Windows XP with the latest Sun Java plugin. It works on both Firefox 3.6 and latest Trunk. I'm not sure if the drag+drop trickery in the Java applet is actually a vulnerability, but Firefox loading a chrome URI definitely is.

Reproducible: Always

Comment 2

[Paul Stone]

Attachment: Testcase

Comment 3

[Paul Stone]

With a slight modification the same trick can be used to perform XSS on a normal website (without the need for chrome URLs). A site would be loaded into the background popup, then then a Javascript URI dropped onto the address bar. It seems that for non-privileged URLs, javascript and data URIs are not automatically loaded when they are dropped onto the address bar. Therefore, the 'Go' button would need to be clicked. This could be achieved by getting the user to double click something in the foreground window. After the first click, the window would be shrunk and the Go button would be under the cursor.

If a fix was made to Java so that the drag gesture could no longer be overridden, then these same vulnerabilities would remain but would require some more complex interaction from the user (real drag movements would be required instead of just clicks)

Comment 4

[Brandon Sterne (:bsterne)]

Interesting.  Dragging a chrome: URL onto a tab works in Windows and Mac, but when I try the same in Linux I get:

Error: uncaught exception: Drop of chrome://browser/content/aboutDialog.xul denied.

Comment 5

[Paul Stone]

Brandon, what's the reasoning for rating this as moderate rather than critical?
The bug allows for arbitrary code execution, 'requiring no user interaction
beyond normal browsing'. It also affects the majority of Firefox users (i.e.
those with Java installed). I believe about 80% of web users have Java
installed, I'm not sure what the percentage is for Firefox users. I also don't
think the testcase 'requires the user to perform complicated and/or unlikely
steps', unless you count the three clicks as complicated.

Comment 6

[Brandon Sterne (:bsterne)]

I did consider the steps required to exploit this bug as somewhat non-standard.  Seeing the attack window appearing and disappearing would certainly alert me (admittedly, not a typical user) that something weird was happening, and I probably wouldn't be compelled to click the second time, dropping the JS URL on the chrome tab.

What you are saying is perfectly reasonable, though.  If others think the severity should be higher, feel free to make it so.

Comment 7

[Lucas Adamski [:ladamski]]

Escalation to chrome: from content is a critical, moderated to high with insufficient reliability of exploit (for example, it doesn't work for me possibly due to my JS settings).  Adjusting to sg:high for now, though I agree that with a really slick exploit it might be even higher.

Comment 10

[Paul Stone]

Attachment: Testcase (v2)

I've attached a second version that removes the need for the second click in the popup. It turns out that Java allows you to initiate a drag-and-drop without any interaction with the applet at all. The mouse doesn't even have to be over the applet. For some reason this doesn't work for the initial drop onto the content area, but it does work for the second drop onto the address bar.

Lucas, if the example doesn't work for you because you have the 'allow javascript to move and resize windows' option disabled, then I suspect it could be made to work by making the foreground window a Java applet window instead of a Firefox one. It would be possible to detect if window.moveBy and window.resizeBy were disabled and use a Java window instead.

Comment 11

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

Seems like we should disallow dropping of URI_INHERITS_SECURITY_CONTEXT URIs.

Comment 12

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

Attachment: patch?

This fixes the testcase. I can't think of any use cases for dragging INHERIT_PRINCIPAL URIs that we care to support, really, but maybe I'm missing something.

I had to change this code to deal with the !sourceDoc case, since otherwise this testcase hit it (presumably because the drag comes from Java) and dragDropSecurity check returned early. I'm not sure if there's a better principal to use than about:blank's, for the no-document case.

Comment 13

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

Note that the DISALLOW_INHERIT_PRINCIPAL change isn't actually needed to fix this - the normal checkLoadURI with sourcePrincipal = about:blank prevents the initial aboutDialog.xul load rather than the second javascript: URI load. So maybe we should just do that and avoid going all the way here.

Comment 14

[Boris Zbarsky [:bzbarsky]]

I'm not sure why we need or don't (and want or don't) the DISALLOW_INHERIT_PRINCIPAL change yet.  Need to think about it a bit.  Can you explain what the net effect of that change is?

The other difference seems to be that you're switching from checkLoadURIStr to checkLoadURIStrWithPrincipal.  This is obviously a good thing to do, but why does it help in this case?  Or is it just the change to not return early if !sourceDoc that helps with that?  The fact that chrome:// could be loaded like this seems like the real issue here, since the file being changed was supposed to prevent just that.

Comment 15

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

The part of that patch that fixes the bug is the change to the !sourceDoc case. With the testcase, the dragSession has no source document, so dragDropSecurityCheck was previously a no-op.

The change to the *WithPrincipal version is unrelated cleanup inspired by the deprecation markings in nsIScriptSecurityManager.idl that I figured I'd do while changing the code - it can be omitted, and probably should be for simplicity's sake (we have bug 327244 tracking that anyways).

The addition of DISALLLOW_INHERIT_PRINCIPAL was based on an early guess about a solution, from before I'd looked into the code. It seems reasonable to me to block drags of such URIs in general. That's probably best split off into a separate bug about a more general belt-and-suspenders approach to these kinds of bugs.

Comment 16

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

Attachment: minimal patch

Comment 17

[Boris Zbarsky [:bzbarsky]]

I'd prefer a separate bug on that, yeah.  Especially since I drag-n-drop data: URIs a good bit.  Usually from the URL bar to my editor once it gets too unwieldy in the url bar.  ;)

The !sourceDoc change is a clear win in my book.

The WithPrincipal change I'm fine with happening in a different bug, but let's get it actually happening.  The existing code is wrong (in the "won't allow drags that it should" sense) in various cases not dealing with web content, for example.

Comment 18

[Mike Beltzner [:beltzner, not reading bugmail]]

Blocks 1.9.2.2.
This alone wouldn't force a 1.9.1.9, but should definitely be taken if it's branch-safe for that series.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Err, it does block 1.9.1.9, not force a 1.9.0.19, but wanted on that branch, too. Durr.

Comment 20

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

(In reply to comment #17)
> I'd prefer a separate bug on that, yeah.  Especially since I drag-n-drop data:
> URIs a good bit.  Usually from the URL bar to my editor once it gets too
> unwieldy in the url bar.  ;)

This check only applies to drops, so the change wouldn't affect that use case, fwiw. I filed bug 547813.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Hmm.  I thought this check was for the drag start.

So with the attached patch dragging a file:// URI from outside the browser into the browser won't work, right?  That seems suboptimal.

Comment 22

[Boris Zbarsky [:bzbarsky]]

On the other hand, it's not clear to me that we want to allow a drag from java to allow loading file://, and that's precisely the "from outside the browser" case...

Comment 23

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

(In reply to comment #21)
> So with the attached patch dragging a file:// URI from outside the browser into
> the browser won't work, right?  That seems suboptimal.

Hmm, indeed :(

I guess there may not currently be a way to differentiate these two cases... So in terms of options for for fixing this testcase, I think that leaves us with pushing for a change in Java (to let us know that the drag is plugin-triggered?), or fixing bug 547813.

Comment 24

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

(In reply to comment #23)
> or fixing bug 547813.

Though that approach would require that we explicitly check URIChainHasFlags(URI_INHERITS_SECURITY_CONTEXT) rather than relying on checkLoadURI, or change the "from" to be something with the system principal (?).

Comment 25

[Boris Zbarsky [:bzbarsky]]

Or using file:/// instead of about:blank as the dummy URI, possibly.

Comment 26

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

(In reply to comment #25)
> Or using file:/// instead of about:blank as the dummy URI, possibly.

Hmm, that's possibly brilliant. My patch with "file:///" rather than about:blank makes file drops work again. Apart from the link-to-file ability, file:/// is the same as about:blank, right? Trying to think of other things this could break.

Comment 27

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

(It also makes it possible for Java-triggered drag/drops to load file:// URIs as you pointed out, of course, but that's much less of a problem than this bug.)

Comment 28

[Boris Zbarsky [:bzbarsky]]

> Apart from the link-to-file ability, file:/// is the same as about:blank, right?

For CheckLoadURI purposes, I believe this is correct.

Comment 29

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

Attachment: patch using file:///

Comment 30

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

I've given this some thought, and I can't think of any ways that using file:/// as the default origin would break anything important. It will block drops of URI_IS_UI_RESOURCE/URI_DANGEROUS_TO_LOAD URIs that were previously allowed in the drag-from-external-app-or-plugin case, but I don't think there are any valid use cases for that, and we kind of need to prevent those to avoid this bug.

I should probably add a comment about the use of "file:///" as the fallback URI here, though. I'm not sure who else should review this patch...

Comment 31

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

Comment on attachment 428294 [details] [diff] [review]
patch using file:///

Dave, can you review this with comment 30 in mind?

Comment 32

[Dave Townsend [:mossop]]

Comment on attachment 428294 [details] [diff] [review]
patch using file:///

This makes sense to me

Comment 35

[Mike Beltzner [:beltzner, not reading bugmail]]

This is reviewed, should it be up for approval?

Comment 39

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

I landed the patch on the trunk:
https://hg.mozilla.org/mozilla-central/rev/66b74d46682e

I used a cover bug (bug 549349) to try and reduce the likelihood of this being exposed as a security bug before we have it fixed on all branches.

Thanks for the clear report and testcase, Paul!

Comment 40

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

(In reply to comment #38)
> Gavin, I noticed from my server logs that you were trying the testcase on
> Linux. It doesn't work particularly well there, since an actual drag gesture is
> needed to complete the drop, instead of just a click.

Indeed, I was trying on Linux. I was aware of the drag gesture difference, but in my case the applet was failing to load for some reason. I think the java plugin I'm using is just broken, though.

Comment 41

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

Comment on attachment 428294 [details] [diff] [review]
patch using file:///

This applies to 1.9.2 cleanly.

Comment 42

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

Attachment: 1.9.0/1.9.1 patch

Comment 43

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 428294 [details] [diff] [review]
patch using file:///

a1922=beltzner

Comment 44

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 430108 [details] [diff] [review]
1.9.0/1.9.1 patch

a19019/1919=beltzner

Comment 45

[Mike Beltzner [:beltzner, not reading bugmail]]

Can we get this landed? Code freeze is Tuesday, March 9th.

Comment 46

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

1.9.2: https://hg.mozilla.org/releases/mozilla-1.9.2/rev/82fa604cdf23
1.9.1: https://hg.mozilla.org/releases/mozilla-1.9.1/rev/ff4a52b1c2a4
1.9.0: mozilla/toolkit/content/nsDragAndDrop.js 	1.11

Comment 47

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

Paul: just want to be clear in case you can't make sense of our Bugzilla flag gibberish: this will be fixed in Firefox 3.0.19, 3.5.9, and 3.6.2 which are all due out shortly. Thanks again!

Comment 48

[Al Billings [:abillings - ex-MoCo]]

I see the same behavior in 1.9.0.18 and my personal debug build of 1.9.0.19pre on Windows XP SP2. The same goes for 1.9.1.8 and 1.9.1.9pre using the attached testcase.

I wind up, eventually, with an alert stating "JS frame :: javascript:alert(Components.stack) :: <TOP_LEVEL> :: line 1" in both instances over a windows loaded from the about firefox chrome as a window after doing the whole "click me" routine on the applet.

This appears fixed for 1.9.2.2 though with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.2pre) Gecko/20100310 Namoroka/3.6.2pre (.NET CLR 3.5.30729) on the same machine. Clicking on the "click me" app does not launch a chrome window.

So, 1.9.0 and 1.9.1 are *not* fixed unless I'm missing something.

Comment 49

[Mike Beltzner [:beltzner, not reading bugmail]]

Paul, can we get you to test the builds for us and let us know if they're fixed for you? You can get them here:

3.0.19pre: ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla1.9.0/
3.5.9pre: ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-1.9.1/
3.6.2pre: ftp://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-1.9.2/

Comment 50

[Al Billings [:abillings - ex-MoCo]]

Was the 1.9.0 and 1.9.1 version of this patch code reviewed? I don't see a reviewer above or was it close enough to the 1.9.2 patch that it didn't matter (though comment 48 might argue against that)?

Comment 51

[Mike Beltzner [:beltzner, not reading bugmail]]

Mossop - you reviewed the original patch. Is the 1.9.0/1.9.1 patch a straight port? I didn't notice the lack of review, and the code looks pretty much the same to me. Gavin's out on vacation otherwise I'd be asking him.

Comment 52

[Mike Beltzner [:beltzner, not reading bugmail]]

(Removing fixed flags for the versions in which Al claims this is not fixed)

Comment 53

[Neil Deakin]

The 1.9.0/1.9.1 is functionally the same. The only difference is that a few lines are moved around due to other changes.

Comment 54

[Paul Stone]

I've tested all three builds that Mike linked to and all are indeed fixed. I tested them on Windows XP SP2, as Al did.

Comment 55

[Mike Beltzner [:beltzner, not reading bugmail]]

Thanks, Paul. Re-marking as FIXED. Al, back to you to verify, maybe use the nightly builds instead of your own local ones?

Comment 56

[Al Billings [:abillings - ex-MoCo]]

I'll check again. The only thing unusual about the builds I was using yesterday is that they were debug builds. I saw no difference in pre and post-fix functionality (each wound up with an alert stating "JS frame ::
javascript:alert(Components.stack) :: <TOP_LEVEL> :: line 1") for 1.9.0 and 1.9.1. This was *not* the case with 1.9.2.

Paul, is the expect behavior that one does *not* get this alert after repeatedly clicking on the "click me" applet?

Comment 57

[Paul Stone]

Al, that's correct. The 'About Firefox' page shouldn't load, and there shouldn't be any alert.

Comment 58

[Al Billings [:abillings - ex-MoCo]]

Attachment: Debug 1.9.0 showing bug occurring

This is my debug build from 3/10/2010. This shows that the testcase still repros in debug. 

In the non-debug nightly on the same machine, the bug is fixed and I never get the chrome window or the alert. That build is Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.19pre) Gecko/2010031005 GranParadiso/3.0.19pre (.NET CLR 3.5.30729).

Comment 59

[Al Billings [:abillings - ex-MoCo]]

Attachment: Debug 1.9.1 showing bug occurring

This is a screenshot from my 1.9.1 debug build from 3/10/2010, showing the bug still occurring with the testcase.

When I use the non-debug 1.9.1 nightly build, the case no longer reproduces. That build is Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100310 Shiretoko/3.5.9pre (.NET CLR 3.5.30729).

Comment 60

[Al Billings [:abillings - ex-MoCo]]

So, Paul is correct in that the non-debug builds will no longer reproduce the bug. My debug builds still do reproduce them and I synched and built them around noon on 3/10/2010 so they have the fix's code. 

I'm not sure if I should mark this as verified fixed for 1.9.0 and 1.9.1 when it still reproduces in debug. I will let Dan Veditz and Mike Beltzner tell me what we want to do here.

Comment 61

[Mike Beltzner [:beltzner, not reading bugmail]]

File a new bug about how the debug builds still trip the behaviour, mark it as blocking this one.

Comment 62

[Boris Zbarsky [:bzbarsky]]

Al, just to double-check what are the changeset IDs (or even just the hg.mozilla.org URIs) your debug builds list in about:buildconfig ?

Comment 63

[Al Billings [:abillings - ex-MoCo]]

Ah crap, my VM blows. I did a rollback and didn't pick up changes so I have pre-fix code. This is my mistake.

I'll go fix my builds now.

Comment 64

[chris hofmann]

Paul, 

If you are interested you are eligible or a security bug bounty on this bug.  You can you contact me at [e-mail] about the bounty.