Closed Bug 395942 (CVE-2007-5045) Opened 17 years ago Closed 17 years ago

QuickTime flaw allows launching default browser with arbitrary parameters on Windows ("quicktime pwns firefox")

Categories

(Core :: General, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: Gavin, Assigned: dveditz)

References

()

Details

(4 keywords, Whiteboard: [sg:critical] mitigation for QT Windows flaw. QT popup crash now bug 396228)

Attachments

(4 files, 3 obsolete files)

This was reported at the URL in the URL field. QuickTime seems to have a flaw that allows specially crafted files handled by the plugin to launch the default browser with arbitrary parameters. The examples on that site use the QuickTime flaw to launch Firefox with "-chrome javascript:<code>" to execute arbitrary code, assuming your default browser is Firefox.

The fix for bug 384384 would protect us in this case if QuickTime used the correct system APIs to launch the default browser, but they appear to read the .html file association from the registry and launch the browser directly.

The QuickTime plugin should be fixed to a) not allow launching the default browser with arbitrary parameters and b) use the right API to launch the default browser (it's not really clear to me why it needs to do that anyways), but perhaps we can "fix" this particular case from our end.

A cross-platform testcase is available at:
http://people.mozilla.com/~gavin/bug/qt/test.mov
It alerts "Vulnerable" if privileged code was allowed to run.
Attached video testcase
Testcase returns "Server not found" page for me in a new tab on Minefield OSX, saying "Firefox can't find the server at www.-chrome%20javascript.com." so this might be Linux/Windows only.

Hooray for 3% of our user base!
Flags: wanted1.8.1.x?
Flags: blocking1.9?
Yeah, it looks like Mac is not affected, and there's no QuickTime plugin for Mac (other plugins that play QuickTime files are unlikely to have the exact same flaw as the official QuickTime plugin on Windows), so this looks to be a Windows-only problem.

My testing was done on Windows with 7.2 and a trunk build, but this appears to affect any recent version of the QuickTime plugin (there's a comment on the blog about 2.0.0.6/QT5.6 not being vulnerable).
OS: All → Windows XP
Hardware: All → PC
I don't know about VLC or xine, but mplayer (all-20061022) doesn't seem to know what to do with this.
OS: Windows XP → All
Hardware: PC → All
(In reply to comment #3)
> and there's no QuickTime plugin for Mac

I meant Linux, of course.
note: they read the .html file association, and then ignore the actual registered command-line template they find (which includes "-osint") in favor of pulling out the executable and then simply appending the "qtnext" value without any of the other specified options.

QuickTime itself is where the bug lies, it does not have to be run as a plugin. If you've got a downloaded local file you get the same effect as long as Firefox is the registered .html (or .htm) handler.

QuickTime ought to be using ShellExecute() instead to get the default browser. If they are paranoid about launching a non-browser (as well they should be) they should vet the scheme against a whitelist of good schemes (http:/https:, maybe ftp:).

Mitigation: uninstall QuickTime. Disabling javascript does not help since chrome ignores that setting.

Protection on our end: drop support for the -chrome argument (adding an enabling pref for use by projects built on our platform, the intended use for -chrome). Regardless of the preceeding, use a whitelist of schemes with the -chrome argument, possibly including only chrome: and resource:.
Assignee: nobody → dveditz
Would it be feasible to completely disable command line parsing, except for one single URL parameter maybe whitelisted on /^(https?|file|ftp):/, in the executable registered as the system-wide "default browser" or in similarly "public" roles, and provide a private command line interface enabled wrapper/alias (e.g. firefox-cli) for compatibility with legacy applications (if any), which would accept the whole story?
Cc'ing Eric in case he can help on the QT side. We have our own side to deal with so this is not a case where one fix on only one "side" suffices.

/be
I'm afraid that removing the -chrome argument entirely is going a bit too far... We're trying to mitigate an issue in another system level app here, so to avoid breaking things unnecessarily I think we should limit ourselves to preventing the specific type of attack that we're seeing. I can understand wanting to be extra sure that no variants are exposed once we've shipped a fix, but whitelisting the protocols that -chrome supports and thereby preventing command-line injection of code seems like it will fix the problem without impacting users of the current flag that rely on being able to point to code that's already on disk (resource:, chrome:, file:, etc). Just doing that would have also fixed bug 384384, right?
(In reply to comment #8)
> We have our own side to deal with so this is not a case where one fix on only
> one "side" suffices.

I'm don't see what on our side would need to be fixed, if QuickTime didn't have this flaw. We can reduce the odds of other broken apps being exploited to use us to execute code by treating any and all command line input as potentially malicious, sure, but I think that's also going too far. I think we should add the minimum number of restrictions on command line input that we need to protect our users, and without bugs in other apps that allow tunneling arbitrary parameters to Firefox, that minimum would be 0.
(In reply to comment #10)
> (In reply to comment #8)
> > We have our own side to deal with so this is not a case where one fix on only
> > one "side" suffices.
> 
> I'm don't see what on our side would need to be fixed, if QuickTime didn't have
> this flaw. We can reduce the odds of other broken apps being exploited to use
> us to execute code by treating any and all command line input as potentially
> malicious, sure, but I think that's also going too far. I think we should add
> the minimum number of restrictions on command line input that we need to
> protect our users, and without bugs in other apps that allow tunneling
> arbitrary parameters to Firefox, that minimum would be 0.

Meanwhile, back in reality, defense in depth is required, sometimes to a degree that compromises a single app's notion of command line utility.

/be
Giorgio's suggestion in comment 7 sounds like a good one to me.  In some ways, it's like a DIY version of what bz and others have suggested that OS vendors should do: provide separate registration for "sandboxed" and "unsandboxed" handlers.
(In reply to comment #11)
> Meanwhile, back in reality, defense in depth is required, sometimes to a degree
> that compromises a single app's notion of command line utility.

Defense in depth doesn't mean "rip out features indiscriminately"; you know that. There isn't only one solution to this problem, and you shouldn't dismiss my argument because I'm suggesting we should be careful before going too far.
(In reply to comment #10)
> We can reduce the odds of other broken apps being exploited to use
> us to execute code by treating any and all command line input as potentially
> malicious, sure, but I think that's also going too far. I think we should add
> the minimum number of restrictions on command line input that we need to
> protect our users, and without bugs in other apps that allow tunneling
> arbitrary parameters to Firefox, that minimum would be 0.

I agree with trying not to break things -- hey, it's NoScript's mission
breaking stuff which Firefox was not brave enough to break ;)

So, what I was trying to say is that there are hooks registered at system level
or otherwise "public" interfaces to be used for inter-process communication
which already assume (more or less explicitly) that we're talking with "a
browser" which accepts only one parameter, i.e. an URL to open.
Exposing the whole set of command line options to these interfaces means
expanding the attack surface with no real advantage: those options won't be
used for any legitimate use, since they're not cross-browser.

So an alternate proposal to my rather radical "firefox" VS "firefox-cli" could
be "firefoxw" VS "firefox", much like "javaw" VS "java": a strict 1 argument
wrapper for public browser hooks, and the normal cli legacy-compatible
executable.
Gavin: where did I write that we should rip out -chrome? Where did I dismiss your argument for whitelisting?

What I objected to in comment 10 was utopian statements of the form "without bugs in other apps that allow tunneling arbitrary parameters to Firefox, that minimum would be 0." Sorry if I was unclear.

/be
(In reply to comment #15)
> Gavin: where did I write that we should rip out -chrome? Where did I dismiss
> your argument for whitelisting?

Apologies, my fault for misinterpreting your comment and reacting defensively.

> What I objected to in comment 10 was utopian statements of the form "without
> bugs in other apps that allow tunneling arbitrary parameters to Firefox, that
> minimum would be 0." Sorry if I was unclear.

I don't deny that it's utopian, but I do think it's true. You said "this is not a case where one fix on only one "side" suffices", and I just wanted to make it clear that were QuickTime able to somehow fix all deployed QuickTime plugins, that fix alone would suffice. Obviously that's not possible, so it wasn't a useful distinction to make, in retrospect.
Attached patch potential patch (1.8 branch) (obsolete) — Splinter Review
Here's one potential patch for Firefox and Thunderbird (we can fix seamonkey the same way on trunk, but will require a different branch patch). This restricts -chrome to chrome: and resource: uris but does not kill the feature outright. People who want to do so can set the new prefs to false. Apps that install additional protocol schemes can add additional prefs if appropriate, and developers who insist on using file: can add that.

We should not support file: for general release because of UNC paths (and obviously not network protocols like http:)

Could have put the whitelisted schemes in a single string pref for compactness but then addons who need to alter it will fight over who sets it last.

This stops the published exploits but does not prevent QuickTime Media-link files from launching actual installed chrome resources. If any of those do bad stuff based on passed arguments then a hole remains. I'm mainly worried about addon chrome since our own chrome should already be avoiding this dangerous practice.

This also does not prevent QuickTime Media-links from calling other arbitrary command-line arguments. http://mxr.mozilla.org/mozilla/search?string=handleflag
We'll need to fix at least -install-global-extension/-theme somehow.
Attachment #280697 - Flags: review?(gavin.sharp)
Comment 7 absolutely sounds like the way to go for me.  Whatever is registered as the default browser should have stringent vetting of input.  The "basic" binary need not do that, however.

Note that the attached patch, as it stands, will break at least the Txul test and the chrome mochitests.  The latter are trunk-only, but the former will send branch orange.
My camara make .mov file so I cant uninstall QuickTime.

So even though we cant stop this exploit being used when QuickTime running as standalone app.

To avoid being vulnerable when we surf using Firefox, or using Thunderbird 
we can disable "QuickTime Plug-in"
at menu Tools > Add-ons > Plugins


I also SeaMonkey for testing (even though Firefox is my default browser)

How do I disable "QuickTime Plug-in" in SeaMonkey?

Flags: blocking1.8.1.7?
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.7?
Flags: blocking1.8.1.7+
Flags: blocking1.8.1.8+
A possible alternative would be to do a URIChainHasFlags check for the URI_INHERITS_SECURITY_CONTEXT flag; this should block all unsafe schemes.
Alternatively, CheckLoadURI with DISALLOW_INHERIT_PRINCIPAL | ALLOW_CHROME ?
Attached patch 1.8 branch patch v2 (obsolete) — Splinter Review
turns out we don't want to block remote "-chrome http:" params, they don't run as privileged code and that feature forms the kernel for web-hosted desktop apps. Need to drop the pref approach and instead just kill the schemes that cause problems -- javascript: and data:

URIChainHasFlags doesn't exist on the 1.8 branch, so we can't use that approach. Would like to use it for the trunk patch though.

Could use checkLoadURI() and the DISALLOW_SCRIPT_OR_DATA flag, but for the branch checking schemes is simpler and avoids a few problems (no handy unprivileged principal, crashes if we end up blocking one I don't have time to debug right now).

The toolkit patch prevents the -install-global-extension/-install-global-theme parameters from being able to install files from remote UNC paths.

The other command-line options Firefox supports are not abusable in the same way. -jsconsole and -new-window could be annoying, for instances, but not dangerous.

-inspector if DOMI is installed looks OK, the URL loaded has no privileges, even javascript: and data:

The JSSH test server if installed could be dangerous, but that's by design and people shouldn't have that installed.

Chatzilla vets arguments to the -chat param. If irc: urls are dangerous then that's a problem from any web page and not just through this QuickTime problem.
Attachment #280697 - Attachment is obsolete: true
Attachment #280849 - Flags: review?(gavin.sharp)
Attachment #280697 - Flags: review?(gavin.sharp)
Comment on attachment 280849 [details] [diff] [review]
1.8 branch patch v2

>+      // check URI scheme against whitelist

I'll fix that comment in the mail handler when I check in, obviously it's a blacklist now. I'll use the wording from the browser handler.
Attached patch trunk versionSplinter Review
Attachment #280857 - Flags: review?(gavin.sharp)
Attachment #280857 - Flags: review?(mscott)
Attachment #280857 - Flags: review?(neil)
Comment on attachment 280857 [details] [diff] [review]
trunk version

On the offchance that you need to change this patch for some reason...

>+const URI_INHERITS_SECURITY_CONTEXT = nsIHttpProtocolHandler
>+                                        .URI_INHERITS_SECURITY_CONTEXT;
Technically it's defined on nsIProtocolHandler ;-) I also wouldn't have minded it all on one line, it's only just over 80 chars and it's pretty obvious what the "missing" chars are :-)

>+        var netutil = Components.classes["@mozilla.org/network/util;1"]
>+                                .getService(nsINetUtil);
I would have named it netUtil, but whatever :-)
Attachment #280857 - Flags: review?(neil) → review+
At least with my simple test cases javascript: doesn't appear to be affected on the branch ("Permission denied to get property UnnamedClass.foo"), only on the trunk; data: is affected on both branch and trunk.
I and many others started using Mozilla to avoid popup and activex.
So please ensure above fix can also stop popups by 
testing using the attached test_popups.html.

test also from Seamonkey when firefox is the default browser.
and also other way arround.

I would a have wished "firefox" VS "firefox-cli" solution 
suggested in comment #14
And leave a power full command line option
>  firefox-cli.exe -chrome javascript:alert('hello from an ext pgm')
as feature for any application to use.
Comment on attachment 280849 [details] [diff] [review]
1.8 branch patch v2

r- per comments on IRC (this scheme checking isn't sufficient).
Attachment #280849 - Flags: review?(gavin.sharp) → review-
Comment on attachment 280857 [details] [diff] [review]
trunk version

>Index: browser/components/nsBrowserContentHandler.js

>+        // only load URIs which do not inherit chrome privs
>         var features = "chrome,dialog=no,all" + this.getFeatures(cmdLine);
>-        openWindow(null, chromeParam, "_blank", features, "");
>+        var uri = resolveURIInternal(cmdLine, chromeParam);
>+        var netutil = Components.classes["@mozilla.org/network/util;1"]
>+                                .getService(nsINetUtil);
>+        if (!netutil.URIChainHasFlags(uri, URI_INHERITS_SECURITY_CONTEXT)) {
>+          openWindow(null, chromeParam, "_blank", features, "");

Maybe use uri.spec, to make sure the that you're actually using the result of the URI parsing (and therefore the URI that passed the security check)?

I only tested the /toolkit and /browser bits, r=me on those.
Attachment #280857 - Flags: review?(gavin.sharp) → review+
Attached patch 1.8 branch patch v3 (obsolete) — Splinter Review
Couldn't make checkLoadURI() work without crashing (null dereference trying to create an exception object on the "current window" since we don't have one). Had to simulate the nested URI support inline.
Attachment #280849 - Attachment is obsolete: true
Attachment #280914 - Flags: review?(gavin.sharp)
Why not use nsIJARURI like the security manager does instead of doing fragile string manipulation?
Bug 396196 filed to cover the launcher/wrapper program idea (comment 7). That approach would require changes to the installer and a lot more tendrils that I'm uncomfortable making in a rush to get this fix out for the branch.

Biju: as a branch firedrill we're not looking to make things perfect, just close off the worst trojan-installing aspects. We do expect Apple to fix QuickTime at some point, this is just to buy time until then.
Attachment #280914 - Attachment is obsolete: true
Attachment #280917 - Flags: review?(gavin.sharp)
Attachment #280914 - Flags: review?(gavin.sharp)
Attachment #280857 - Flags: approval1.9?
Flags: blocking1.9? → blocking1.9+
Attachment #280917 - Flags: review?(gavin.sharp) → review+
Comment on attachment 280917 [details] [diff] [review]
1.8 branch patch v4 (use nsIJARURI)

approved for 1.8.1.7/1.8.1.8, a=dveditz
Attachment #280917 - Flags: approval1.8.1.8+
Attachment #280917 - Flags: approval1.8.1.7+
Leaving this list of URIs as sec-sensitive for now.

I tested before/after for the branch and trunk patches against the list of URIs at:
http://people.mozilla.com/~gavin/bug/qt/urilist.txt

using:

firefox.exe -chrome <uri>

Summary:

should be blocked:
jar:data:
jar:jar:data:
javascript:alert(Components.stack);
data:

shouldn't get chrome privs:
file:
http:

should still work (get chrome privs):
chrome:
Checked into MOZILLA_1_8_BRANCH and GECKO181_20070712_RELBRANCH for 1.8.1.8 and 1.8.1.7
Whiteboard: [sg:critical] interaction with quicktime on Windows
Attached patch XPFE patchSplinter Review
PFE startup is weird. Apparently the way to distinguish the chrome commandline handler is that it handles args but doesn't open the window with them.
Attachment #280995 - Flags: superreview?(dveditz)
Attachment #280995 - Flags: review?(jag)
According to http://docs.info.apple.com/article.html?artnum=305149 this problem shouldn't be happening regardless of the broken way in which the default browser is launched. qtnext is supposed to be limited to http: and https: urls (last advisory on the page).
Attachment #280995 - Flags: review?(jag) → review+
i used the Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7) Gecko/2007091417 Firefox/2.0.0.7 Firefox 2007 RC2 for some tests and http://www.gnucitizen.org/projects/0day-quicktime-pwns-firefox/ 

None of this testcases works for me in Firefox 2.0.0.7rc2. Only a second browser window is launched but no other application like calc or so.
The additional browser window is expected. If the -chrome argument is rejected then the browser will behave as if it's not there at all, generally opening a new window unless other arguments say to do something else.
Probably too late, but you could also include another binary or script specifically for this purpose (launching a browser from external sources), and for double measure make it check for the arguments that the registry specifies and bomb otherwise.
(In reply to comment #43)
> Probably too late, but you could also include another binary or script
> specifically for this purpose (launching a browser from external sources), and
> for double measure make it check for the arguments that the registry specifies
> and bomb otherwise.

You probably mean Comment 7 and Comment 14, which Comment 34 morphed into Bug 396196.

So is the "trunk version" patch still waiting on reviews?  Or does it just need approval to land?
Comment on attachment 280857 [details] [diff] [review]
trunk version

Already blocking -> approval not needed. mscott's review is pending.
Attachment #280857 - Flags: approval1.9?
verified fixed 1.8.1.7 after testsruns with the exploit testcases on http://www.gnucitizen.org/projects/0day-quicktime-pwns-firefox/

Tested the normal behavior when Firefox is loaded and also launching firefox with the testcases

-> adding verified keyword 
Comment on attachment 280995 [details] [diff] [review]
XPFE patch

sr=dveditz

a=dveditz to land this seamonkey only change on the branches.
Attachment #280995 - Flags: superreview?(dveditz)
Attachment #280995 - Flags: superreview+
Attachment #280995 - Flags: approval1.8.1.8+
Attachment #280995 - Flags: approval1.8.1.7+
Trunk version is just waiting for a non-busted tree to land. I keep missing the windows
(In reply to comment #46)
> Already blocking -> approval not needed.

According to the tree rules at the time ("threat level red") is was. Apparently the rules change day to day just to keep us on our toes.

Checking in browser/components/nsBrowserContentHandler.js;
/cvsroot/mozilla/browser/components/nsBrowserContentHandler.js,v  <--  nsBrowserContentHandler.js
new revision: 1.45; previous revision: 1.44
done
Checking in mail/components/nsMailDefaultHandler.js;
/cvsroot/mozilla/mail/components/nsMailDefaultHandler.js,v  <--  nsMailDefaultHandler.js
new revision: 1.9; previous revision: 1.8
done
Checking in suite/browser/nsBrowserContentHandler.js;
/cvsroot/mozilla/suite/browser/nsBrowserContentHandler.js,v  <--  nsBrowserContentHandler.js
new revision: 1.6; previous revision: 1.5
done
Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.255; previous revision: 1.254
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
verified fixed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007091804 Minefield/3.0a8pre and the exploit testcases - same results as for 1.8.1.7 see comment #47

- Changing Bug to verified
Status: RESOLVED → VERIFIED
Comment on attachment 280857 [details] [diff] [review]
trunk version

not sure if you still need this but it looks good to me Dan.
Attachment #280857 - Flags: review?(mscott) → review+
now attachment 280881 [details] test_popups.html   
is making firefox crash ir run CPU at near 100%

bug report
----------

BuildID: 2007091804
CrashTime: 1190164012
Email: bijumaillist@gmail.com
InstallTime: 1190163434
ProductName: Firefox
SecondsSinceLastCrash: 16
URL: https://bugzilla.mozilla.org/attachment.cgi?id=280881
UserID: b337f90c-beb0-4dbd-98ee-dca08d81381b
Vendor: Mozilla
Version: 3.0a8pre

This report also contains information about the state of the application when it crashed.
(In reply to comment #54)
> now attachment 280881 [details] test_popups.html   
> is making firefox crash ir run CPU at near 100%
> 
thats bug 396228, but its not a regression of this bug/patch, has crashed also before the patch was checked in.
(In reply to comment #55)
> thats bug 396228, but its not a regression of this bug/patch,
> has crashed also before the patch was checked in.

Thanks for info
Biju's popup issue and crashing testcase moved to bug 396228. I've hidden the attachment here to avoid commingling the issues.
Summary: QuickTime flaw allows launching default browser with arbitrary parameters ("quicktime pwns firefox") → QuickTime flaw allows launching default browser with arbitrary parameters on Windows ("quicktime pwns firefox")
Whiteboard: [sg:critical] interaction with quicktime on Windows → [sg:critical] mitigation for QT Windows flaw. QT popup crash now bug 396228
GP a8(M8) RC2 is released before official a8 ?
pal-moz: not entirely sure what you mean, but discussion not about fixing the bug in question does not belong in bugzilla. Please find our developer forums. Obviously our builds are available to testers in the "nightly" test directories before we actually release them, otherwise they would be released untested.

This bug is not fixed in the "M8" release today. The code-freeze for M8 was before this bug was filed. The bug is fixed in Firefox 2.0.0.7 and Firefox 3 nightlies.
(In reply to comment #59)
> This bug is not fixed in the "M8" release today. The code-freeze for M8 was
> before this bug was filed. The bug is fixed in Firefox 2.0.0.7 and Firefox 3
> nightlies.
> 

I know.
if M8 (the build including this bug) will be released, M8 will be a risky/dangerous build ?
so RC2 with this fix should be out/tested, then release as an official M8 build ?
M8 will be released with this flaw unfixed, M9 will come in a few weeks. Alpha builds are not intended for daily use, they are technology previews so people can play with the new features and do some compatibility testing. Anyone using the build on a daily or regular basis is a "tester", and testers should be using nightly builds (which will have the fix) rather than infrequent alpha builds.
isn't running with user supplied http chrome risky?
(In reply to comment #62)
>isn't running with user supplied http chrome risky?
Unlike data, http doesn't gain privileges from running as -chrome
XPFE patch checked in to the branch.
(In reply to comment #64)
> XPFE patch checked in to the branch.

Seems to have broken Camino on the 1.8 branch -- the 1.8 branch Camino tinderboxes are unfortunately at http://tinderbox.mozilla.org/Camino/ .
Sorry, the patch was missing a change to Makefile.in; I've fixed that now.
Group: security
Group: security
verified fixed 1.8.1.8 using the testcases and Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.8pre) Gecko/20070927 BonEcho/2.0.0.8pre ID:2007092704

Same results as for the 1.8.1.7 build (Comment #47)

-> Adding the 1.8.1.8 verified keyword
Alias: CVE-2007-5045
Ever confirmed: true
Depends on: 412434
Comment on attachment 280917 [details] [diff] [review]
1.8 branch patch v4 (use nsIJARURI)

a=asac for 1.8.0.15
Attachment #280917 - Flags: approval1.8.0.15+
Comment on attachment 280995 [details] [diff] [review]
XPFE patch

a=asac for 1.8.0.15
Attachment #280995 - Flags: approval1.8.0.15+
Committed attachments 280917 and 280995 to the 1.8.0 branch
Keywords: fixed1.8.0.15
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: