Closed Bug 384384 (IDEF2595) Opened 17 years ago Closed 17 years ago

Multiple Vendor Multiple Browser Interaction URI Handler Command Injection Vulnerability

Categories

(Toolkit :: Startup and Profile System, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dveditz, Assigned: robert.strong.bugs)

References

()

Details

(Keywords: verified1.8.1.5, Whiteboard: [sg:critical] (aka SA25984))

Attachments

(8 files, 11 obsolete files)

(deleted), text/html
Details
(deleted), text/plain
Details
(deleted), text/html
Details
(deleted), patch
benjamin
: review-
Details | Diff | Splinter Review
(deleted), patch
benjamin
: review+
Details | Diff | Splinter Review
(deleted), patch
mscott
: review+
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
(deleted), patch
Details | Diff | Splinter Review
iDefense reports a security problem that arises from a combination of IE7 and Firefox 2. The attached PoC was unreadable (working on getting a copy) but the description sounds like Thor Larholm's Safari-for-Windows exploit blogged at http://larholm.com/2007/06/12/safari-for-windows-0day-exploit-in-2-hours/ and is presumably similar. Excerpt: - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - II. DESCRIPTION Remote exploitation of an input handling vulnerability in the handling of external application URIs by multiple browsers could allow remote code execution as the local user. A remote attacker can cause an Internet Explorer user, with Mozilla Firefox installed, to execute arbitrary commands. By creating an iFrame with a specially constructed link, attackers may execute arbitrary programs and files may be downloaded, uploaded, or modified as the current user. This vulnerability is due to the interaction between programs. Internet Explorer does not escape double-quote characters in some URLs. This can allow a specially constructed link to be interpreted by Firefox as multiple arguments. III. ANALYSIS Exploitation of this vulnerability allows an attacker to execute arbitrary commands as the current user. To exploit this vulnerability, the attacker must create a website containing a maliciously constructed link. The victim must have Firefox installed, but not currently running. When Internet Explorer requests a frame with a 'firefoxurl:' URI, Internet Explorer will launch Firefox with the arguments supplied from the SRC (source) parameter. When opening a URL, Firefox is started with the following command line: [path/to/firefox.exe] -url "%1" -requestPending The "%1" is replaced with the source URL. If the URL contains a double-quote character followed by a space, the quoting will be closed, and the rest of the source URL will be treated as new arguments. By supplying the '-chrome' (user interface) option, followed by JavaScript that calls XPCOM interface functions, an attacker can execute arbitrary commands. XPCOM (Cross Platform Component Object Model) is the framework used by Mozilla-based products that allow the same base code to be used for multiple applications on multiple platforms. The XPCOM user interface code for Firefox accesses the underlying code on the system, providing a standardized interface across operating systems. Code interacting with the browser at this level can perform any operation the user running the browser can. This vulnerability is related to a similar vulnerability in the Beta version of Apple's Safari Web browser for Windows. In Vista with IE 7 in Protected Mode, which is the default setting, a dialog will ask you to confirm that you want to run Firefox. IV. DETECTION iDefense has confirmed the existence of this vulnerability to be exploitable with Firefox version 2.0.0.4 from Internet Explorer 7 on Windows XPSP2. Previous versions may also be affected. While this vulnerability is due to Internet Explorer not escaping double-quotes in the URL passed to external programs, the actual code execution comes from Firefox. The interaction between the two programs is the source of this vulnerability. V. WORKAROUND iDefense is currently unaware of any workaround for this issue, other than keeping Firefox running while browsing the Web with Internet Explorer. iDefense Labs is researching other potential workarounds and will update this report should any become available. [...] IX. CREDIT This vulnerability was discovered by Greg MacManus (iDefense Labs).
Flags: wanted1.8.1.x?
Flags: blocking1.9?
Flags: blocking1.8.1.5?
As mentioned I haven't see the PoC, but assuming it's like Larholm's I have to wonder why we allow javascript: urls with the -chrome command-line argument, or really anything but an actual chrome: url. But we shouldn't focus on -chrome alone, there could be other problematic command-line arguments now or in the future we should protect. Perhaps require -url to be the last command-line argument if used and ignore any options that follow.
Alias: IDEF2595
Does Firefox really register a "firefoxurl:" protocol? That's not very nice; it prevents users from safely using other browsers when Firefox has a known hole.
Summary: Multiple Vendor Multiple Browser Interaction URI Handler Command Injection Vulnerability → Vulnerability involving IE passing URLs containing spaces to Firefox
Given that the attacker is already executing commands on the target box, is it really worth changing Firefox to make it harder to use for this? Won't they just use some other app to do their dirty work instead? Jesse: I don't _think_ we do... http://lxr.mozilla.org/mozilla/search?string=firefoxurl Gerv
We allow opening web content with the -chrome flag because it's a common operation on intranets. If anything, the bug is that the javascript URL gets chrome privs instead of about:blank privs, but I don't know how we pick a principal in that case. Really though this has to be a windows/IE bug... there are many other ways this could be exploited given the number of commandline options we support, and they are obviuosly passing us bogus data.
(In reply to comment #3) > Given that the attacker is already executing commands on the target box, The attacker is not, they have a web page with a URL in it. It happens to be something that will cause IE to use an external handler (Larholm's Safari PoC used gopher:) and Firefox is the registered handler for that scheme. IE and Safari apparently are not escaping quotes properly, but Firefox accepts arguments wholly inappropriate for handling remote URLs. > Is it really worth changing Firefox to make it harder to use for this? Won't > they just use some other app to do their dirty work instead? Some other hypothetical handler that will turn an innocent text string into code execution? I doubt it. If they find a different vulnerability, though, hackers will definitely try that instead (a while back it was a buffer overrun in a default rtsp: handler; QuickTime I think, but maybe Real). (In reply to comment #2) > Does Firefox really register a "firefoxurl:" protocol? We register HKLM/SOFTWARE/Classes/FirefoxURL with a shell/open/command subkey and then set the values of ftp, gopher, http, and https to FirefoxURL under HKLM/SOFTWARE/Clients/StartMenuInternet/FIREFOX.EXE/Capabilities/URLAssociations This, amazingly, does cause firefoxurl: to be sent to Firefox, which then doesn't recognize it and loops trying to open it in an external app which turns out to be Firefox. Definitely do NOT want to check the "remember" box on _that_ one! We can stop the looping by setting network.protocol-handler.external.firefoxurl to false, but it's too late by then since the malicious -chrome command-line has already executed. We seem to be the only browser setting the "Capabilities" key under StartMenuInternet, at least on WinXP -- is it a Vista thing? This was added for bug 354005 If it's not a Vista requirement we should save our placeholders in a new place, because anything under HKR with a shell/open/command can be used as a scheme. (e.g. giffile://something will attempt to launch IE) 1) why do we register firefoxurl ? 2) even if we didn't, the problem still exists if Firefox is the default browser. Other browsers will slurp up http(s)/ftp regardless of default handler, but we've still got gopher: sitting out there.
I'd prefer to keep the original advisory title for the summary because people will eventually be searching for this once the advisory is public. "passing spaces" isn't really the problem, it's the quotes.
Summary: Vulnerability involving IE passing URLs containing spaces to Firefox → Multiple Vendor Multiple Browser Interaction URI Handler Command Injection Vulnerability
Attached file PoC (open in IE with FF not running) (deleted) —
(In reply to comment #5) >... > We seem to be the only browser setting the "Capabilities" key under > StartMenuInternet, at least on WinXP -- is it a Vista thing? This was added for > bug 354005 This is a Vista requirement and we set it so that if the OS is upgraded to Vista from a previous OS we need these keys under HKLM for Vista integration. MS did a similar implementation for local files (see FirefoxHTML) a while back and added this for protocol handlers with Vista. > If it's not a Vista requirement we should save our placeholders in a new place, > because anything under HKR with a shell/open/command can be used as a scheme. > (e.g. giffile://something will attempt to launch IE) The Capabilities and FirefoxURL keys are under HKLM. You are seeing the abstracted HKCR which is a combination of HKLM and HKCU Software\Classes. > 1) why do we register firefoxurl ? For Vista Integration.
(In reply to comment #8) > The Capabilities and FirefoxURL keys are under HKLM. Yes, I mentioned that. > You are seeing the abstracted HKCR which is a combination of > HKLM and HKCU Software\Classes. Which is what windows uses to determine the "active" handlers, should some user have customized HKCU settings. > > 1) why do we register firefoxurl ? > For Vista Integration. Vista requires that there be a way to launch any specific browser even if the user doesn't want that browser handling stuff by default? Does IE do the same thing on Vista (IE7 doesn't in WinXP)? Yowza, that means even if you've switched to Firefox people could try to launch IE exploits. Yes, we'll put up a dialog first but that's pretty thin protection. Please let us know what key name IE uses, I'd like to set network.protocol-handler.external.<ieurl> to false in the default prefs to protect against this. Safari and Opera, too, if they do this on Vista.
> We register HKLM/SOFTWARE/Classes/FirefoxURL with a shell/open/command subkey > This, amazingly, does cause firefoxurl: to be sent to Firefox, which then Does this mean that any Named key under HKLM/SOFTWARE/Classes automatically becomes a protocol handler? That sounds like a pretty serious bug in Vista to me: There are many keys there that have nothing to do with URI protocols. That's a huge (and pretty stupid) attack vector. On my system IE has "htmlfile" even when Firefox is my default browser. Setting .external = false on whackamole bad handlers sounds like a bad way to deal with this.
Attached file IE's http regkey (deleted) —
(In reply to comment #9) > (In reply to comment #8) > > The Capabilities and FirefoxURL keys are under HKLM. > > Yes, I mentioned that. That was for clarification in regards to (In reply to comment #5) > If it's not a Vista requirement we should save our placeholders in a new place, > because anything under HKR with a shell/open/command can be used as a scheme. > (e.g. giffile://something will attempt to launch IE) and to point out that the statement of "we should save our placeholders in a new place" wouldn't work. The system level default value must be under HKLM\Software\Classes which is then abstracted to HKCR unless it is over-ridden by a value under HKCU\Software\Classes. > > You are seeing the abstracted HKCR which is a combination of > > HKLM and HKCU Software\Classes. > > Which is what windows uses to determine the "active" handlers, should some user > have customized HKCU settings. yes > > > 1) why do we register firefoxurl ? > > For Vista Integration. > > Vista requires that there be a way to launch any specific browser even if the > user doesn't want that browser handling stuff by default? No, the purpose is to have a system level default for protocol handlers in the same way as earlier versions of windows expect a system level default for file handlers. > Does IE do the same thing on Vista (IE7 doesn't in WinXP)? yes > Yowza, that means even if you've > switched to Firefox people could try to launch IE exploits. Yes, we'll put up a > dialog first but that's pretty thin protection. > > Please let us know what key name IE uses, I'd like to set > network.protocol-handler.external.<ieurl> to false in the default prefs to > protect against this. Safari and Opera, too, if they do this on Vista. For protocols IE uses IE.FTP IE.HTTP IE.HTTPS
(In reply to comment #10) > > We register HKLM/SOFTWARE/Classes/FirefoxURL with a shell/open/command subkey > > > This, amazingly, does cause firefoxurl: to be sent to Firefox, which then > > Does this mean that any Named key under HKLM/SOFTWARE/Classes automatically > becomes a protocol handler? That sounds like a pretty serious bug in Vista to > me: There are many keys there that have nothing to do with URI protocols. > That's a huge (and pretty stupid) attack vector. No. With Vista protocol handlers have a HKLM\Software\Classes key similar to how file handlers had a value there. There must also be a value name of URL Protocol for it to be a URL protocol. > On my system IE has "htmlfile" even when Firefox is my default browser. > > Setting .external = false on whackamole bad handlers sounds like a bad way to > deal with this. >
rstrong said he'd look at the command handler here.
Assignee: nobody → robert.bugzilla
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Whiteboard: [sg:moderate] (critical, but requires IE plus FF installed.)
Billy Rios of VeriSign independently reported similar abuse. At the following URL he has fun with various command-line options we support, several of which work even if Firefox is currently running. http://www.xs-sniper.com/sniperscope/Cross-Browser-Scripting.html
following up to comment 13, we're thinking that given an indication that "this is a call via a registered protocol/mimetype handler" we would only honor the -url commandline option (and perhaps a small whitelisted set of others). We already register these handlers with -requestPending and that option has little use outside that context so we may be able to use that as our signal. But we'll have to reorder the options so that comes first before any potential bad quoting.
deveditz, I believe that nothing should be using firefoxurl as an -url param so how about if we just close that off completely by not handling any command line arguments when that is present instead of trying to handle the url and not handling specific arguments, etc.? btw: the url in comment #14 is no longer available.
Attached patch patch (obsolete) (deleted) — Splinter Review
dveditz, this patch implements what I suggested in comment #16 and does the same thing for firefoxhtml while we are there. Does this seem like a reasonable approach?
Attachment #269050 - Flags: review?(dveditz)
Attached patch patch - use a regexp (obsolete) (deleted) — Splinter Review
Changed to use a regexp. I don't think we need the check for firefoxhtml and will remove it of desired.
Attachment #269050 - Attachment is obsolete: true
Attachment #269054 - Flags: review?(dveditz)
Attachment #269050 - Flags: review?(dveditz)
This patch doesn't handle the gopher / safari case. To properly handle urls when we need to restart during startup We remove requestPending so we can't use that to differentiate. We could do one of the following 1) add a new flag when we remove requestPending to handle that case 2) remove the gopher protocol reg keys when it points to our app 3) other? There has been some discussion about removing the gopher protocol reg keys previously which is the direction I think we should take. Also, I'm not entirely certain that the case when the app is already open and DDE is used isn't also vulnerable... I'll verify that tomorrow.
(In reply to comment #19) >... > Also, I'm not entirely certain that the case when the app is already open and > DDE is used isn't also vulnerable... I'll verify that tomorrow. When using DDE I get the same results and we don't pass requestPending or any other flags via DDE so just using a command line flag isn't an option. The current patch already fixes launching with command line flags and DDE and the only additional thing that would need to be done is the removal of the gopher protocol reg keys.
Attached file Billy Rios testcase (obsolete) (deleted) —
The URL from comment 14 was moved to http://www.xs-sniper.com/sniperscope/IE-Pwns-Firefox.html and made public at http://sla.ckers.org/forum/read.php?3,12752,12752#msg-12752 I'm attaching the testcase in case it moves again. For the record, that's not exactly what "Mozilla Security" (me) told him: "Thanks for sending the details. Ultimately we view this as bad behavior on the other app's part (as in the Safari bug Thor Larholm blogged about) but we are pragmatically working on protecting users from this on our end for a future security update. The hook you used was added for Vista compatibility and you will no doubt start seeing many more apps adding similar features." the "hook" referred to his use of firefoxurl: which I didn't want to explicitly mention in unencrypted mail.
Outlook Express on WinXP SP2 has the same problem as IE, it'll launch these links from an HTML email. I wonder if the programs are simply calling ShellExecute() and trusting that to escape properly. That's probably the level it should be fixed at since that's the API constructing the command-line. Outlook Express _does_ escape http: and ftp: urls so those are not an attack vector. But it does not escape the unknown firefoxurl: scheme. rstrong reports that the default "WinMail" on Vista does _not_ have this problem so maybe they added escaping. Opera 9.2 is safe, it escapes quotes.
Attached patch patch rev 3 (obsolete) (deleted) — Splinter Review
Attachment #269054 - Attachment is obsolete: true
Attachment #269054 - Flags: review?(dveditz)
Comment on attachment 269241 [details] [diff] [review] patch rev 3 Seth, since this has a couple of installer changes could you review the entire patch? Thanks!
Attachment #269241 - Flags: review?(sspitzer)
Whiteboard: [sg:moderate] (critical, but requires IE plus FF installed.) → [sg:critical] (for IE & Outlook Express users with FF installed)
-chrome does work whether the browser is running or not on WinXP, I must have had a bug in my own testcase, but Billy's works just fine :-(
Attachment #269120 - Attachment is obsolete: true
Comment on attachment 269241 [details] [diff] [review] patch rev 3 This patch seems incomplete. I'm glad we're blocking firefoxurl: as an obvious sign of evilness, but I strongly feel we should do what I suggest in comment 15. That is (spelled out) 1) reorder the installed handlers so -requestPending is first (so it can't be messed with by injected content) 2) if we find -requestPending then we know we're a registered command-line handler and we MUST ignore all further options other than -url. caveats: if -requestPending has other uses then we should invent a new option whose sole purpose is to signal this "I'm a registered handler" state. rationale: IE happens to correctly escape http(s)/ftp/gopher URLS before (presumably) sending them to ShellExecute() so the injection trick isn't going to work, but that's no guarantee all other apps will do it. I really don't want to end up vulnerable later because acrobat/flash/aim/pidgin/<random RSS reader>/some-IRC-client get it wrong and don't escape http: urls, leading to the exact same -chrome exploit through a protocol we can't simply say is evil and not support. Also people could install firefox extentions that register additional protocol handlers that IE does not escape (for example, Chatzilla could register irc: and then here we are again)
As expected, the exploit works from within Firefox if https://addons.mozilla.org/en-US/firefox/addon/1419 or similar is installed. NetScape should be vulnerable as well for obvious reasons, but I didn't check. In the meanwhile, both the chrome privileged and the "normal" XSS tricks are prevented by the current NoScript development version, available from http://noscript.net/getit#devel and scheduled to be released in the mainstream tomorrow.
(In reply to comment #26) > (From update of attachment 269241 [details] [diff] [review]) > This patch seems incomplete. I'm glad we're blocking firefoxurl: as an obvious > sign of evilness, but I strongly feel we should do what I suggest in comment > 15. That is (spelled out) > > 1) reorder the installed handlers so -requestPending is first (so it can't be > messed with by injected content) > > 2) if we find -requestPending then we know we're a registered command-line > handler and we MUST ignore all further options other than -url. > > caveats: if -requestPending has other uses then we should invent a new option > whose sole purpose is to signal this "I'm a registered handler" state. > > rationale: IE happens to correctly escape http(s)/ftp/gopher URLS before > (presumably) sending them to ShellExecute() so the injection trick isn't going > to work, but that's no guarantee all other apps will do it. I really don't want > to end up vulnerable later because acrobat/flash/aim/pidgin/<random RSS > reader>/some-IRC-client get it wrong and don't escape http: urls, leading to > the exact same -chrome exploit through a protocol we can't simply say is evil > and not support. Also people could install firefox extentions that register > additional protocol handlers that IE does not escape (for example, Chatzilla > could register irc: and then here we are again) I've been considering this as well. The caveat you stated regarding requestPending is entirely true as stated in comment #20 so a new flag will have to be used. On top of that we will most likely need to modify the reg key for DDE in order to prefix it with the new command line flag.
The approach I plan on taking for evaluating whether or not the command line is valid is to reorder the flags so they are -requestPending -newFlag -url "%1". The dde request will also include this new flag though I haven't decided yet the best way to implement this. If -newFlag (actual name not yet decided upon) is present only allow the -requestPending flag and the -url flag followed by the url param in the command line. If there are additional flags the entire operation will be a noop. Though we could just remove the additional flags I think this way is safer since the only time we should end up in this state is when an exploit is attempted. Also, I think we should continue to make firefoxurl as a noop when it is passed in as the url param since this will prevent the app looping when it meets the aforementioned criteria above (e.g. -url "firefoxurl:http://localhost/").
-untrusted ?
note: to cover all of the cases when launching the app with arguments we are going to have to do this at startup since several command lines are handled in nsAppRunner.cpp
Attachment #269241 - Attachment is obsolete: true
Attachment #269241 - Flags: review?(sspitzer)
> -untrusted ? Or some varation on "OS Integration" since that's what these are for? -osint -osi -os ? If we register anything similar on Mac and Linux we should add the same flag to prevent future issues on those OS's. How does Linux know to open Firefox when I click on a local .html file?
Attached patch branch patch ver 4 (obsolete) (deleted) — Splinter Review
Benjamin, this prevents invoking command line arguments in nsAppRunner if the -osint command line flag is present except for -help, -version, etc. since those return immediately. It also adds nsAllowedCheckCommandLineHandler to nsBrowserContentHandler.js with a category of a-allowedcheck so it should be called first. Not pretty but it should be safe. For the trunk I think a new interface for validation should be added (e.g. nsICommandLineValidator) which would allow commandlinehandlers to opt in to be validators. It would be enumerated in the same way nsICommandLineHandler is enumerated and have one method for the validation of the commandline args. This would allow extension provided commandlinehandlers that add os integration to validate their arguments. We would still need the checks for -osint in nsAppRunner since it handles several commandline flags early in startup. Thoughts?
Attachment #270052 - Flags: review?(benjamin)
Attached patch branch patch rev 5 (fixed a typo) (obsolete) (deleted) — Splinter Review
Attachment #270052 - Attachment is obsolete: true
Attachment #270067 - Flags: review?(benjamin)
Attachment #270052 - Flags: review?(benjamin)
(In reply to comment #33) >... > For the trunk I think a new interface for validation should be added (e.g. > nsICommandLineValidator) which would allow commandlinehandlers to opt in to be > validators. It would be enumerated in the same way nsICommandLineHandler is > enumerated and have one method for the validation of the commandline args. This > would allow extension provided commandlinehandlers that add os integration to > validate their arguments. We would still need the checks for -osint in > nsAppRunner since it handles several commandline flags early in startup. I've got this working and just need to finish up the code comments... I'm happy to say that the trunk patch may very well be safe enough for the branch.
Attached patch trunk patch rev 4 (obsolete) (deleted) — Splinter Review
Benjamin, I think this patch is just as safe for the branch as the previous patch.
Attachment #270130 - Flags: review?(benjamin)
Attached patch trunk patch rev 5 (comment fix) (obsolete) (deleted) — Splinter Review
Attachment #270130 - Attachment is obsolete: true
Attachment #270131 - Flags: review?(benjamin)
Attachment #270130 - Flags: review?(benjamin)
Attached patch patch rev 6 (diff from trunk) (obsolete) (deleted) — Splinter Review
Benjamin, I'd like to go this route for both the trunk and branch
Attachment #270067 - Attachment is obsolete: true
Attachment #270131 - Attachment is obsolete: true
Attachment #270136 - Flags: review?(benjamin)
Attachment #270067 - Flags: review?(benjamin)
Attachment #270131 - Flags: review?(benjamin)
Attached patch Thunderbird patch in progress (obsolete) (deleted) — Splinter Review
Scott, the logic in validate in nsMailDefaultHandler.js needs to be beefed up due to having to handle both a mail and compose flag. The MAPI calls will also have to be verified that it sets state to STATE_REMOTE_EXPLICIT.
Attached patch patch rev 6 (diff from branch) (deleted) — Splinter Review
Forgot to include the new idl in the trunk patch. This has been tested using attachment #269305 [details] and firefoxurl:http://localhost
Attachment #270136 - Attachment is obsolete: true
Attachment #270213 - Flags: review?(benjamin)
Attachment #270136 - Flags: review?(benjamin)
Comment on attachment 270213 [details] [diff] [review] patch rev 6 (diff from branch) Are there any instances where CheckArg should work with "osint"? Can we just change the implementation of CheckArg to always check for "osint" and return ARG_BAD if it is present? That means we'd have to teach a few callers about ARG_BAD, but it would be less confusing than checking for "osint" in each case.
Attachment #270213 - Flags: review?(benjamin) → review-
I considered that and went with this so the stderr output would be correct... I'll submit a new patch shortly implementing the check for osint in CheckArg. Thanks!
Attached patch patch rev 7 (diff from branch) (obsolete) (deleted) — Splinter Review
Implements the changes requested in comment #41. Regretfully attachment #270213 [details] [diff] [review] seems to be cleaner especially in regards to STDERR as this patch since there are a couple of one off's in how the arguments are handled.
Attachment #271103 - Flags: review?(benjamin)
Attached patch patch rev 7 (diff from branch) (deleted) — Splinter Review
Missed a one off case in the previous patch.
Attachment #271103 - Attachment is obsolete: true
Attachment #271110 - Flags: review?(benjamin)
Attachment #271103 - Flags: review?(benjamin)
Whiteboard: [sg:critical] (for IE & Outlook Express users with FF installed) → [sg:critical] need r=bsmedberg (for IE & Outlook Express users with FF installed)
I'm removing the security sensitive bit, it seems that http://larholm.com/2007/07/10/internet-explorer-0day-exploit/ exists now. note that I've stuck the URL into the URL field to avoid duplicates. this is not to say that I'm giving larholm any more credit for the find.
How pervasive is the use of -chrome on Intranets? I can't find any references to general usage of it, so perhaps it could simply be disabled by default with a preferences flag - then Intranets can enable it specifically. For what it's worth, simply removing the "URL Protocol" value from the registry will prevent IE and any other applications from launching Firefox through FirefoxURL on XPSP2, without breaking additional functionality. I don't know about Vista, but it should still work just fine as the URL handler logic is not used for opening html files and the DefaultIcon is still in place.
Whiteboard: [sg:critical] need r=bsmedberg (for IE & Outlook Express users with FF installed) → [sg:critical] need r=bsmedberg (for IE & Outlook Express users with FF installed) SA25984
timeless: should not have opened the iDefense advisory information without coordinating release with them. They didn't publish when Billy Rios re-invented this a couple weeks ago, for example.
Group: security
Comment on attachment 271110 [details] [diff] [review] patch rev 7 (diff from branch) >Index: toolkit/xre/nsAppRunner.cpp >+ ar = CheckArg("a", PR_FALSE, &temp); For this and -profile, why do you pass PR_FALSE? >+ /* nsICommandLineValidator */ >+ validate : function bch_validate(cmdLine) { >+ // Other handlers may use osint so only handle the osint flag if the url >+ // flag is also present and the command line is valid. >+ var osintFlagIdx = cmdLine.findFlag("osint", false); >+ var urlFlagIdx = cmdLine.findFlag("url", false); >+ if (urlFlagIdx > -1 && (osintFlagIdx > -1 || >+ cmdLine.state == nsICommandLine.STATE_REMOTE_EXPLICIT)) { >+ var urlParam = cmdLine.getArgument(urlFlagIdx + 1); >+ if (cmdLine.length != urlFlagIdx + 2 || /firefoxurl:/.test(urlParam)) >+ throw NS_ERROR_ABORT; >+ cmdLine.handleFlag("osint", false) Does this handle URLs which have correctly-encoded spaces in them? e.g. in some app: http://example.com/some%20file.html Does this get passed to Firefox as some%20file.html or some file.html?
Attachment #271110 - Flags: review?(benjamin) → review+
Whiteboard: [sg:critical] need r=bsmedberg (for IE & Outlook Express users with FF installed) SA25984 → [sg:critical] please keep bug closed until iDefense announcement (SA25984)
Attached patch Thunderbird patch (deleted) — Splinter Review
Attachment #270140 - Attachment is obsolete: true
Attachment #271754 - Flags: review?(mscott)
(In reply to comment #48) > (From update of attachment 271110 [details] [diff] [review]) > >Index: toolkit/xre/nsAppRunner.cpp > > >+ ar = CheckArg("a", PR_FALSE, &temp); > > For this and -profile, why do you pass PR_FALSE? Actually, all the CheckArg calls in HandleRemoteArgument should pass PR_FALSE since the command line check that calls this passes PR_TRUE. > >+ /* nsICommandLineValidator */ > >+ validate : function bch_validate(cmdLine) { > >+ // Other handlers may use osint so only handle the osint flag if the url > >+ // flag is also present and the command line is valid. > >+ var osintFlagIdx = cmdLine.findFlag("osint", false); > >+ var urlFlagIdx = cmdLine.findFlag("url", false); > >+ if (urlFlagIdx > -1 && (osintFlagIdx > -1 || > >+ cmdLine.state == nsICommandLine.STATE_REMOTE_EXPLICIT)) { > >+ var urlParam = cmdLine.getArgument(urlFlagIdx + 1); > >+ if (cmdLine.length != urlFlagIdx + 2 || /firefoxurl:/.test(urlParam)) > >+ throw NS_ERROR_ABORT; > >+ cmdLine.handleFlag("osint", false) > > Does this handle URLs which have correctly-encoded spaces in them? e.g. > > in some app: http://example.com/some%20file.html > > Does this get passed to Firefox as some%20file.html or some file.html? It gets passed quoted via the reg entry value of: <path to firefox.exe> -requestPending -osint -url "%1" where %1 is replaced by the url.
Comment on attachment 271754 [details] [diff] [review] Thunderbird patch r/sr=mscott. I'm still working on putting together a build with the patches but the code looks good to me. Thanks Rob.
Attachment #271754 - Flags: review?(mscott) → review+
Checked in to Trunk and MOZILLA_1_8_BRANCH Trunk Checking in mozilla/toolkit/components/commandlines/public/Makefile.in; /cvsroot/mozilla/toolkit/components/commandlines/public/Makefile.in,v <-- Makefile.in new revision: 1.3; previous revision: 1.2 done RCS file: /cvsroot/mozilla/toolkit/components/commandlines/public/nsICommandLineValidator.idl,v done Checking in mozilla/toolkit/components/commandlines/public/nsICommandLineValidator.idl; /cvsroot/mozilla/toolkit/components/commandlines/public/nsICommandLineValidator.idl,v <-- nsICommandLineValidator.idl initial revision: 1.1 done Checking in mozilla/toolkit/components/commandlines/src/nsCommandLine.cpp; /cvsroot/mozilla/toolkit/components/commandlines/src/nsCommandLine.cpp,v <-- nsCommandLine.cpp new revision: 1.16; previous revision: 1.15 done Checking in mozilla/toolkit/xre/nsAppRunner.cpp; /cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v <-- nsAppRunner.cpp new revision: 1.178; previous revision: 1.177 done Checking in mozilla/browser/components/nsBrowserContentHandler.js; /cvsroot/mozilla/browser/components/nsBrowserContentHandler.js,v <-- nsBrowserContentHandler.js new revision: 1.41; previous revision: 1.40 done Checking in mozilla/browser/components/shell/src/nsWindowsShellService.cpp; /cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v <-- nsWindowsShellService.cpp new revision: 1.47; previous revision: 1.46 done Checking in mozilla/browser/installer/windows/nsis/shared.nsh; /cvsroot/mozilla/browser/installer/windows/nsis/shared.nsh,v <-- shared.nsh new revision: 1.9; previous revision: 1.8 done Checking in mozilla/mail/components/nsMailDefaultHandler.js; /cvsroot/mozilla/mail/components/nsMailDefaultHandler.js,v <-- nsMailDefaultHandler.js new revision: 1.8; previous revision: 1.7 done Checking in mozilla/mail/installer/windows/nsis/shared.nsh; /cvsroot/mozilla/mail/installer/windows/nsis/shared.nsh,v <-- shared.nsh new revision: 1.6; previous revision: 1.5 done Checking in mozilla/mail/components/shell/nsMailWinIntegration.cpp; /cvsroot/mozilla/mail/components/shell/nsMailWinIntegration.cpp,v <-- nsMailWinIntegration.cpp new revision: 1.9; previous revision: 1.8 done MOZILLA_1_8_BRANCH Checking in mozilla/toolkit/components/commandlines/public/Makefile.in; /cvsroot/mozilla/toolkit/components/commandlines/public/Makefile.in,v <-- Makefile.in new revision: 1.2.12.1; previous revision: 1.2 done Checking in mozilla/toolkit/components/commandlines/public/nsICommandLineValidator.idl; /cvsroot/mozilla/toolkit/components/commandlines/public/nsICommandLineValidator.idl,v <-- nsICommandLineValidator.idl new revision: 1.1.2.1; previous revision: 1.1 done Checking in mozilla/toolkit/components/commandlines/src/nsCommandLine.cpp; /cvsroot/mozilla/toolkit/components/commandlines/src/nsCommandLine.cpp,v <-- nsCommandLine.cpp new revision: 1.13.2.1; previous revision: 1.13 done Checking in mozilla/toolkit/xre/nsAppRunner.cpp; /cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v <-- nsAppRunner.cpp new revision: 1.113.2.21; previous revision: 1.113.2.20 done Checking in mozilla/browser/components/nsBrowserContentHandler.js; /cvsroot/mozilla/browser/components/nsBrowserContentHandler.js,v <-- nsBrowserContentHandler.js new revision: 1.12.2.22; previous revision: 1.12.2.21 done Checking in mozilla/browser/components/shell/src/nsWindowsShellService.cpp; /cvsroot/mozilla/browser/components/shell/src/nsWindowsShellService.cpp,v <-- nsWindowsShellService.cpp new revision: 1.21.2.6; previous revision: 1.21.2.5 done Checking in mozilla/browser/installer/windows/nsis/shared.nsh; /cvsroot/mozilla/browser/installer/windows/nsis/shared.nsh,v <-- shared.nsh new revision: 1.3.2.8; previous revision: 1.3.2.7 done Checking in mozilla/mail/components/nsMailDefaultHandler.js; /cvsroot/mozilla/mail/components/nsMailDefaultHandler.js,v <-- nsMailDefaultHandler.js new revision: 1.4.4.4; previous revision: 1.4.4.3 done Checking in mozilla/mail/installer/windows/nsis/shared.nsh; /cvsroot/mozilla/mail/installer/windows/nsis/shared.nsh,v <-- shared.nsh new revision: 1.2.2.5; previous revision: 1.2.2.4 done Checking in mozilla/mail/components/shell/nsMailWinIntegration.cpp; /cvsroot/mozilla/mail/components/shell/nsMailWinIntegration.cpp,v <-- nsMailWinIntegration.cpp new revision: 1.2.2.4; previous revision: 1.2.2.3 done
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8.1.5
Resolution: --- → FIXED
Verified using test cases in comment #14, with installation of Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5) Gecko/2007071317 Firefox/2.0.0.5 Also Thunderbird version 2.0.0.5 (20070716) with modified test cases in comment #14
Blocks: 389257
Attached file (deleted) —
After reading the following http://msinfluentials.com/blogs/jesper/archive/2007/07/10/blocking-the-firefox-gt-ie-0-day.aspx I realized we also need to remove the handler added by the MS shim.
Attachment #273430 - Flags: review?(sspitzer)
Attachment #273430 - Attachment is obsolete: true
Attachment #273430 - Flags: review?(sspitzer)
Attached file (deleted) —
Attachment #273433 - Flags: review?(sspitzer)
Comment on attachment 273433 r=sspitzer
Attachment #273433 - Flags: review?(sspitzer) → review+
Comment 56 needs to be in a new follow-up bug. We've already shipped a release with what we thought was the fix, adding more blocking/fixed flags here would make a big mess. Filed bug 389287 to track that installer patch.
Attached file (deleted) —
After discussing this with sspitzer we decided this would be a simpler approach. Carrying forward his r+
Attachment #273433 - Attachment is obsolete: true
Attachment #273435 - Flags: review+
Attachment #273435 - Flags: approval1.8.1.7?
Attachment #273435 - Attachment is obsolete: true
Attachment #273435 - Flags: review+
Attachment #273435 - Flags: approval1.8.1.7?
I would really like to open this, as this is discussed widely in the open, and I can't reference the patch and bug.
whiteboard says [sg:critical] please keep bug closed until iDefense announcement (SA25984) http://secunia.com/advisories/25984/ is public, so I assume this can be opened now.
Yeah, but then this morphed into a new unfixed bug at comment 56 :-(
Attachment #273430 - Attachment is private: true
Attachment #273433 - Attachment is private: true
Attachment #273435 - Attachment is private: true
Group: security
Whiteboard: [sg:critical] please keep bug closed until iDefense announcement (SA25984) → [sg:critical]
Whiteboard: [sg:critical] → [sg:critical] (aka SA25984)
Blocks: 389613
Flags: in-testsuite?
Blocks: 418150
Product: Core → Core Graveyard
What does it mean that this is in "Core Graveyard" now? We still have this code, and could conceivably make a similar mistake in the future that would be a security hole we'd have to fix. How is that a "Graveyard"?
Component: Cmd-line Features → Startup and Profile System
Product: Core Graveyard → Toolkit
QA Contact: cmd-line → startup
The cmd-line handling component is obsolete. I moved all open bugs in this component to the correct component and had the component moved to graveyard so that new bugs could not be opened in it.
Blocks: 1574396
No longer blocks: 1574396
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: