Last Comment Bug 663899 - .jar should not be openable on Mac or Linux, download only (CVE-2011-3666)
: .jar should not be openable on Mac or Linux, download only (CVE-2011-3666)
Status: VERIFIED FIXED
[sg:critical][qa-examined-192][qa!]
: qawanted, verified-aurora, verified-beta
Product: Core Graveyard
Classification: Graveyard
Component: File Handling (show other bugs)
: unspecified
: x86 Mac OS X
: -- critical (vote)
: mozilla9
Assigned To: Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
:
:
Mentors:
https://bugzilla.mozilla.org/attachme...
Depends on: 689195
Blocks: CVE-2011-2372 704622
  Show dependency treegraph
 
Reported: 2011-06-13 10:59 PDT by Daniel Veditz [:dveditz]
Modified: 2016-06-22 12:16 PDT (History)
21 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
-
wontfix
-
wontfix
+
fixed
+
fixed
+
fixed
-
wontfix
.23+


Attachments
Simple runnable jar file to test with (18.72 KB, application/java-archive)
2011-06-22 09:45 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
WIP for feedback (2.17 KB, patch)
2011-06-22 10:24 PDT, Cameron Kaiser [:spectre]
smichaud: feedback+
Details | Diff | Splinter Review
Simple WebStart testcase (2.28 KB, text/html)
2011-06-22 15:28 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Simple Python script test case (1.04 KB, text/x-python)
2011-06-22 15:53 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
Mark .jar and .air as executable regardless of execute bit (2.29 KB, patch)
2011-06-22 20:10 PDT, Cameron Kaiser [:spectre]
smichaud: review+
asa: approval‑mozilla‑aurora+
christian: approval1.9.2.20+
Details | Diff | Splinter Review
Simple shell script testcase (24 bytes, application/x-sh)
2011-06-23 15:12 PDT, Steven Michaud [:smichaud] (Retired)
no flags Details
use a dummy file everywhere. (1.55 KB, patch)
2011-08-25 20:07 PDT, Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
smichaud: review+
jst: approval‑mozilla‑aurora+
jst: approval‑mozilla‑beta+
christian: approval1.9.2.23+
Details | Diff | Splinter Review

Description Daniel Veditz [:dveditz] 2011-06-13 10:59:14 PDT
+++ This bug was initially created as a clone of Bug #662309 +++

On windows it was simple to make .jar executable in nsLocalFileWin and force users to download a .jar rather than letting them "open" it and run a privileged local Java Application. On Mac most files with the executable bit set do trigger the "save only" path, but something is special-casing .jar (noticing that the system has a handler for it) and letting it be opened anyway. I could not find any equivalent "blacklist" on Mac that .jar could be added to, and we desperately need one: just because the OS knows how to run a java archive (or other types) doesn't mean it's wise to do so from a web page.

Someone who knows mac will have to take this. The original bug on this was externally reported and the Windows variant is fixed in Fx5 and 3.6.18 -- this is important to fix soon.
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2011-06-13 11:49:44 PDT
> but something is special-casing .jar

Not at all.  It's just that it does not have the executable bit set...

The same issue probably arises with other filetypes on Mac and Linux if the default OS handler for the type can do dangerous things (and for non-executable types like Word documents with macros on Windows, for that matter!).
Comment 2 :Ehsan Akhgari 2011-06-13 12:44:51 PDT
(In reply to comment #1)
> > but something is special-casing .jar
> 
> Not at all.  It's just that it does not have the executable bit set...
> 
> The same issue probably arises with other filetypes on Mac and Linux if the
> default OS handler for the type can do dangerous things (and for non-executable
> types like Word documents with macros on Windows, for that matter!).

So, should _we_ special case .jar files on Mac?
Comment 3 christian 2011-06-13 14:37:55 PDT
We are willing to ship Firefox 5 with the incomplete stopgap.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-06-15 22:36:55 PDT
> So, should _we_ special case .jar files on Mac?

Perhaps.  If so, also various other stuff too, right?  E.g. what's the (default?) OS helper app for .pl files?  .py files?  .sh files?  Other interesting things?
Comment 5 :Ehsan Akhgari 2011-06-16 13:02:28 PDT
(In reply to comment #4)
> > So, should _we_ special case .jar files on Mac?
> 
> Perhaps.  If so, also various other stuff too, right?  E.g. what's the
> (default?) OS helper app for .pl files?  .py files?  .sh files?  Other
> interesting things?

That sounds like a catch-me-if-you-can game :/  Do we have a better way to recognize these types of files in general (not that I know of any)?
Comment 6 Josh Aas 2011-06-21 21:04:31 PDT
I'd really rather not have this assigned to me - perhaps Steven Michaud can look into it?
Comment 7 Cameron Kaiser [:spectre] 2011-06-22 06:44:51 PDT
Is it merely as simple as, as in the Windows patch, having nsLocalFileUnix::IsExecutable return PR_FALSE for these and other files such as scripts? It seems like this would be useful for other Unices. If that seems an acceptable approach, I'll whip up something.
Comment 8 Cameron Kaiser [:spectre] 2011-06-22 06:47:39 PDT
er, PR_TRUE even
Comment 9 Steven Michaud [:smichaud] (Retired) 2011-06-22 08:02:32 PDT
> Is it merely as simple as, as in the Windows patch, having
> nsLocalFileUnix::IsExecutable return PR_FALSE for these and other
> files such as scripts? It seems like this would be useful for other
> Unices. If that seems an acceptable approach, I'll whip up
> something.

> er, PR_TRUE even

This sounds reasonable to me, at least as a stopgap.

I'll test other browsers' behavior with *.jar and *.py files (plus
other files that the OS supports on some level, but which we don't
want to run from the browser), and dig up what I can about how the OS
determines how these files should be handled.  But this will probably
take a while.
Comment 10 Steven Michaud [:smichaud] (Retired) 2011-06-22 09:09:41 PDT
Note that whatever we do, we (presumably) need to ensure that WebStart applications still work properly.
Comment 11 Steven Michaud [:smichaud] (Retired) 2011-06-22 09:45:01 PDT
Created attachment 541078 [details]
Simple runnable jar file to test with
Comment 12 Steven Michaud [:smichaud] (Retired) 2011-06-22 10:06:25 PDT
Just now I've done some very basic testing (with various browsers on
OS X 10.5.8) with my TestJavaApp.jar testcase.

When you click on the file, FF 4.0.1 and a current nightly put up a
dialog asking whether to open the file (with JarLauncher) or save it
to disk.  Opening the file (with JarLauncher) is the default.

Safari automatically saves it to disk.

Chrome and Opera also put up dialogs asking the user to open or save
the file, but both of them default to saving it.
Comment 13 Cameron Kaiser [:spectre] 2011-06-22 10:24:54 PDT
Created attachment 541099 [details] [diff] [review]
WIP for feedback

First pass for feedback (this builds, not tested). Steven, what do you think?
Comment 14 Steven Michaud [:smichaud] (Retired) 2011-06-22 13:04:38 PDT
Comment on attachment 541099 [details] [diff] [review]
WIP for feedback

This looks fine to me, with the following caveats:

1) You don't need to include "app" in executableExts.

   Things that end in *.app are directory trees, not files -- they're
   application "bundles".

   I'm not sure if we ever download directory trees (as opposed to
   their individual contents).  If we do, it's probably best to call
   the nsILocalFileMac IsPackage() method to make sure we're not
   dealing with a "bundle" (application bundle or otherwise) -- which
   we'd presumably never want to "execute", only to "save".

2) We might need to add other extensions to executableExts, like "py".

I'm working to create a hello-world WebStart app and Python script to
test with.  This is just to start with -- we may also need other kinds
of testcases.

And by the way, a *.jar file isn't a "bundle" as that word is usually
used with respect to OS X -- it's basically a zip archive, which is a
single file.
Comment 15 Cameron Kaiser [:spectre] 2011-06-22 13:36:10 PDT
No, I'm aware that .app should be always a directory bundle, but I put that there because if it really is a file and it ends in .app, we should probably always treat it as hinky.

The other extensions I was thinking of: .py, .pl, .sh, .rb
Any others?

I'll run off a build and bang on it tonight and then make a reviewable patch (I'm in a conference call on the office PC right now).
Comment 16 Steven Michaud [:smichaud] (Retired) 2011-06-22 14:39:05 PDT
> No, I'm aware that .app should be always a directory bundle, but I
> put that there because if it really is a file and it ends in .app,
> we should probably always treat it as hinky.

To keep our sanity, I think we should only put extensions in
executableExts that we *know* will cause trouble, and then only if
there isn't some other way to exclude them (such as using
nsILocalFileMac::IsPackage()).

> The other extensions I was thinking of: .py, .pl, .sh, .rb

What is .rb?

And once gain, I think we should only include extensions that we can
demonstrate (with a testcase) are "executable" when we ask the OS to
"run" files that have them.

> Any others?

.csh?  .pm?

Unless we limit ourselves to extensions that we know might get
"executed", the list might become almost endless.

As I said above, I think the approach you suggested in comment #7 is
at best a stopgap.  I'm going to try to find a way to make FF and/or
the OS no longer ever default to wanting to "run" an "executable" file
(as opposed to "saving" it).

I think it would be acceptable if the user were offered the choice of
running or saving a file, as long as the default was always to save
the file.  Especially since this is what Chrome and Opera do.  What do
people think?
Comment 17 Steven Michaud [:smichaud] (Retired) 2011-06-22 15:28:40 PDT
Created attachment 541197 [details]
Simple WebStart testcase
Comment 18 Steven Michaud [:smichaud] (Retired) 2011-06-22 15:53:06 PDT
Created attachment 541209 [details]
Simple Python script test case
Comment 19 Steven Michaud [:smichaud] (Retired) 2011-06-22 15:57:16 PDT
> Simple Python script test case

Firefox doesn't default to "opening" this file, and doesn't even know
what app to "open" it with.  And I can't get Safari, Chrome or Python
to execute it, either.

So we might *not* need to include any script file extensions in
executableExts.
Comment 20 Steven Michaud [:smichaud] (Retired) 2011-06-22 15:59:12 PDT
Comment on attachment 541209 [details]
Simple Python script test case

By the way, I copied this from http://www.neowin.net/forum/topic/706942-howto-program-using-python/.
Comment 21 Steven Michaud [:smichaud] (Retired) 2011-06-22 16:11:00 PDT
(Following up comment #10)

> And I can't get Safari, Chrome or Python to execute it, either.

I meant Safari, Chrome or *Opera*.
Comment 22 Cameron Kaiser [:spectre] 2011-06-22 20:10:48 PDT
Created attachment 541270 [details] [diff] [review]
Mark .jar and .air as executable regardless of execute bit

(taking)

This is essentially the same patch, minus the .app. On testing, the helper dialogue for .jar comes up with Save File as the default, and if you try to double click it from the Downloads window, it correctly warns you the file is executable. Steven, are you okay with reviewing? Technically this isn't just Mac, but I can #ifdef it to just Mac if this is desirable. However, I suspect this is valuable for other platforms.
Comment 23 Steven Michaud [:smichaud] (Retired) 2011-06-23 08:26:06 PDT
Comment on attachment 541270 [details] [diff] [review]
Mark .jar and .air as executable regardless of execute bit

This looks fine to me.

I'm going to spend another few hours trying to find a more generic way
to fix this bug ... but I doubt that whatever I find (if anything)
will be better than your patch.  It really helps that we (apparently)
don't need to worry about executable scripts (more on this in a later
comment).

> Technically this isn't just Mac, but I can #ifdef it to just Mac if
> this is desirable. However, I suspect this is valuable for other
> platforms.

Testing on a couple of Linux distros (Ubuntu and Fedora), I found that
Firefox didn't want to execute my runnable jarfile testcase on either
of them.  So I was tempted to ask you to #ifdef this patch for the
Mac.

But your patch won't (I don't think) change Firefox's behavior on
either of these Linux distros, and (who knows) there might exist a
Linux equivalent of OS X's Jar Launcher, which (if installed) Firefox
might (by default) want to use to "open" a downloaded jarfile.

So I'm fine with having this patch effect any platform with a
Unix-like filesystem.  But let's keep an eye out for possible
problems.
Comment 24 Cameron Kaiser [:spectre] 2011-06-23 09:22:15 PDT
Comment on attachment 541270 [details] [diff] [review]
Mark .jar and .air as executable regardless of execute bit

Thanks for the review, Steven :)

I don't think there should be a problem with this approach for 3.6, but I'll try this tonight. Meanwhile requesting approvals for -aurora (7) and -beta (6), assuming it lands after the beta 5->6 transition on 7/5.
Comment 25 Steven Michaud [:smichaud] (Retired) 2011-06-23 09:51:56 PDT
Comment on attachment 541270 [details] [diff] [review]
Mark .jar and .air as executable regardless of execute bit

By the way, I think this should bake on the trunk for a while (a week?).  So it should land there as soon as possible.
Comment 26 Cameron Kaiser [:spectre] 2011-06-23 09:56:58 PDT
Check-in needed, unless you're able to, Steven (I don't have access).
Comment 27 :Ehsan Akhgari 2011-06-23 12:43:30 PDT
Landed on mozilla-inbound.
Comment 28 Steven Michaud [:smichaud] (Retired) 2011-06-23 15:12:17 PDT
Created attachment 541520 [details]
Simple shell script testcase
Comment 29 Steven Michaud [:smichaud] (Retired) 2011-06-23 16:00:15 PDT
OS X makes executable scripts very difficult to run except from the
command line.  So, for that matter, do the Linux distros that I
tested.  So it's very unlikely that we'll have to worry about
executable scripts in the context of this bug.

I've posted a couple of testcases here (a Python script and a shell
script) and tested using Firefox or other browsers to download them --
I couldn't get Firefox (or any other browser) to execute either
script.

Both scripts (of course) run (and work correctly) from the command
line.  But double-clicking on the shell script loads it into TextEdit,
and double-clicking on the Python script runs a program called Build
Applet, which creates a double-clickable wrapper for the Python
script.

There's a wonderfully useful "Default Apps" pref panel that tells you
a lot about the information that Launch Services stores, and uses to
decide which apps to run in which circumstances
(http://www.rubicode.com/Software/RCDefaultApp/).  With it I couldn't
find a single script extension or mime type that was associated with a
program that could run it.  (Many script extensions or mime types are
simply absent from the LS database.  Others (like 'pl', 'rb' or 'tcl')
are associated with a text editor or an IDE (like XCode).)
Comment 30 Cameron Kaiser [:spectre] 2011-06-23 20:02:44 PDT
On 1.9.2 the fix also performs as expected, but there are two different ::IsExecutable's, one for Solaris and BeOS, and one for everyone else. Do we want the change in both places or just in the everyone else category? (I imagine the latter.)
Comment 31 neil@parkwaycc.co.uk 2011-06-24 03:34:03 PDT
Merged from mozilla-inbound to mozilla-central.

http://hg.mozilla.org/mozilla-central/rev/83c0640ea8b1
Comment 32 Asa Dotzler [:asa] 2011-06-29 11:57:45 PDT
Comment on attachment 541270 [details] [diff] [review]
Mark .jar and .air as executable regardless of execute bit

Please land this asap. Thanks.
Comment 33 Steven Michaud [:smichaud] (Retired) 2011-06-29 12:53:00 PDT
Comment on attachment 541270 [details] [diff] [review]
Mark .jar and .air as executable regardless of execute bit

Landed on aurora:
http://hg.mozilla.org/releases/mozilla-aurora/rev/210714b766cd
Comment 34 Cameron Kaiser [:spectre] 2011-06-29 14:00:46 PDT
What's the call on comment 30?
Comment 35 Johnny Stenback (:jst, jst@mozilla.com) 2011-07-07 13:54:48 PDT
Marking fixed for branches where this was fixed!
Comment 36 christian 2011-08-01 10:13:28 PDT
(In reply to comment #30)
> On 1.9.2 the fix also performs as expected, but there are two different
> ::IsExecutable's, one for Solaris and BeOS, and one for everyone else. Do we
> want the change in both places or just in the everyone else category? (I
> imagine the latter.)

Everyone else is fine.
Comment 37 Cameron Kaiser [:spectre] 2011-08-01 10:51:45 PDT
I'll generate a 1.9.2 patch for that code section as soon as I get my dev systems back online (house move this weekend and Time Warner sucks).
Comment 38 christian 2011-08-01 11:35:50 PDT
It actually transplanted successfully with a bit of context fuzz:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2bf4a50fa874

I didn't build locally so hopefully the transplant didn't bork anything....I'll watch tinderbox.
Comment 39 Al Billings [:abillings] 2011-08-09 13:15:43 PDT
(In reply to Steven Michaud from comment #11)
> Created attachment 541078 [details]
> Simple runnable jar file to test with

Running this in 3.6.19 and 3.6.20 build 1 on OS X, I see no difference in behavior. Both of them attempt to "open" by Jar Launcher, rather than offering saving as the only option.

This doesn't strike me as "fixed" behavior unless I'm completely misunderstanding Dan's comments in comment 0.
Comment 40 Steven Michaud [:smichaud] (Retired) 2011-08-09 13:25:50 PDT
> Running this in 3.6.19 and 3.6.20 build 1 on OS X, I see no
> difference in behavior. Both of them attempt to "open" by Jar
> Launcher, rather than offering saving as the only option.

Does this happen with a clean profile?

I'll download 3.6.20 and see what happens to me.
Comment 41 Steven Michaud [:smichaud] (Retired) 2011-08-09 13:27:37 PDT
Where is "3.6.20 build 1"?
Comment 42 Steven Michaud [:smichaud] (Retired) 2011-08-09 13:33:10 PDT
Since this bug's fix was landed on the 1.9.2 branch on 2011-08-01, it is (of course) not in FF 3.6.19, which is dated 2011-07-07.
Comment 43 Cameron Kaiser [:spectre] 2011-08-09 13:35:25 PDT
The patch isn't supposed to make saving the only option, just the default. I'm still completely hosed netwise at my house, but when I built a previous 1.9.2 with this, that's what it did for me.
Comment 44 Al Billings [:abillings] 2011-08-09 13:47:02 PDT
(In reply to Steven Michaud from comment #41)
> Where is "3.6.20 build 1"?

ftp://ftp.mozilla.org/pub/firefox/nightly/3.6.20-candidates/build1/

This wasn't a completely clean profile. It had the Jar Launcher as the default option.
Comment 45 Al Billings [:abillings] 2011-08-09 13:48:30 PDT
On a completely clean profile (using OS X 10.7), when I go to https://bugzilla.mozilla.org/attachment.cgi?id=541078 with Firefox 3.6.20 build 1, the default option is "Jar Launcher" there.
Comment 46 Steven Michaud [:smichaud] (Retired) 2011-08-09 14:14:05 PDT
I see the same thing, testing with today's trunk nightly and a clean profile, also on OS X 10.7.

But if you do Save As even once with my TestJavaApp.jar file, all subsequent tests work properly -- you're prompted to save the file.  This is even if you don't ask the browser to "do this automatically from now on".

Sounds like we screwed up rather badly :-(

I'm in the midst of dealing with two other very urgent bugs (bug 668953 and bug 670842), and I *really* don't want to have to drop them for what might turn out to be a week, in order to fix this bug properly.

So I suggest we hold fixing this bug until FF 3.6.21.

I don't think we need to back out Cameron's patch, though.
Comment 47 Al Billings [:abillings] 2011-08-09 14:16:01 PDT
So this isn't really fixed for Firefox 6/7 or 1.9.2 then. 

We need to make sure not to disclose this accidentally until we can get a final fix.
Comment 48 Steven Michaud [:smichaud] (Retired) 2011-08-09 14:20:17 PDT
> So this isn't really fixed for Firefox 6/7 or 1.9.2 then.

Apparently not ... though I haven't had a chance to do more than minimal testing.  The key is to always test with a clean profile.

Thanks for catching this, Al ... though it was a bit like a punch in the stomach :-(
Comment 49 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-08-09 14:27:50 PDT
(In reply to Al Billings [:abillings] from comment #47)
> So this isn't really fixed for Firefox 6/7 or 1.9.2 then. 

Do we have confirmation of that for Firefox 6/7? It's possible that the 1.9.2 patch alone was broken, and I don't see anyone mentioning tests on 6/7.
Comment 50 Steven Michaud [:smichaud] (Retired) 2011-08-09 14:29:20 PDT
Can someone else please do those tests?
Comment 51 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-08-09 14:30:22 PDT
Oh. Bug 571193 didn't land on 1.9.2, so the patch would have only affected Linux there (and not Mac). Downside to blind transplants :(
Comment 52 Al Billings [:abillings] 2011-08-09 14:34:42 PDT
(In reply to Gavin Sharp from comment #49)
> (In reply to Al Billings [:abillings] from comment #47)
> > So this isn't really fixed for Firefox 6/7 or 1.9.2 then. 
> 
> Do we have confirmation of that for Firefox 6/7? It's possible that the
> 1.9.2 patch alone was broken, and I don't see anyone mentioning tests on 6/7.

Well, Steven said he tested on "trunk" though I'm not sure what build he used.
Comment 53 Steven Michaud [:smichaud] (Retired) 2011-08-09 14:39:30 PDT
Today's mozilla-central nightly:  firefox-2011-08-09-03-07-51-mozilla-central.
Comment 54 Cameron Kaiser [:spectre] 2011-08-09 16:28:51 PDT
I'll look at this in more detail as soon as I can get Time Warner to actually do anything.
Comment 55 Steven Michaud [:smichaud] (Retired) 2011-08-10 08:20:21 PDT
(Following up comment #50)

> (In reply to Al Billings [:abillings] from comment #47)
>> So this isn't really fixed for Firefox 6/7 or 1.9.2 then.
>
> Do we have confirmation of that for Firefox 6/7? It's possible that
> the 1.9.2 patch alone was broken, and I don't see anyone mentioning
> tests on 6/

> Can someone else please do those tests?

Seeing as I'm (for now) no longer in the middle of other things, and
nobody else has done them, I've now done them myself.

I've tested FF 3.6b5, yesterday's mozilla-central nightly (again), and
today's aurora nightly -- this time all on OS X 10.6.8.  I gave myself
a clean profile before the first of these tests.  In all of them this
bug still happens (testing with my "simple runnable jar file" testcase
(attachment 541078 [details])).
Comment 56 Daniel Veditz [:dveditz] 2011-08-10 16:12:14 PDT
(In reply to Al Billings [:abillings] from comment #47)
> So this isn't really fixed for Firefox 6/7 or 1.9.2 then. 
> 
> We need to make sure not to disclose this accidentally until we can get a
> final fix.

We need to unset the status flags then because release triage and announcement decisions are based on them.

(In reply to Cameron Kaiser from comment #43)
> The patch isn't supposed to make saving the only option, just the default.

What would it take to make saving the only option? That's what happens for "unsafe/executable" types on windows and that's what we want to happen on other platforms. See "not openable....download only" in bug summary!
Comment 57 Sheila Mooney 2011-08-17 10:49:18 PDT
Not sure where we are in getting this fixed. It seems a bit stalled. Steven are you the right guy to help with this?
Comment 58 Cameron Kaiser [:spectre] 2011-08-17 11:04:18 PDT
Here's another thought. Have HelperApps.updateTypeInfo in toolkit/mozapps/downloads/content/helperApps.js willfully "lie": if the file is in the list of proscribed extensions, either by using IsExecutable() in NSPR or having its own list, then set entry.appPath to "" and entry.saveToDisk to true. That should only present one option, if I understand the code correctly.
Comment 59 Steven Michaud [:smichaud] (Retired) 2011-08-17 11:34:59 PDT
> Steven are you the right guy to help with this?

I *could* help with this -- I'm very good at figuring out obscure
problems, especially on the Mac.  But I'm not currently very familiar
with the code in which changes would probably need to be made.	So
it'd take me a while to catch up (a week sounds about right), and I've
got *lots* of (probably) more urgent bugs to deal with -- mostly
having to do with OS X 10.7.

So if you can find someone else	to shepherd this bug, I'd really
appreciate it :-)

Cameron, unfortunately I can't really respond to your suggestion
without spending a lot more time digging in to this bug.
Comment 60 Sheila Mooney 2011-08-18 13:41:17 PDT
JP, can you help us find an owner for this bug?
Comment 61 JP Rosevear [:jpr] 2011-08-19 12:31:12 PDT
After reviewing this and running some simple Mac tests, is there a reason we can't use the Windows type approach for this right now based on extensions?

Rafael may have some time to look too.
Comment 62 Johnny Stenback (:jst, jst@mozilla.com) 2011-08-25 13:27:08 PDT
Ok, Rafael, can you look into this? We need traction on this very soon, so if you can't, please speak up.
Comment 63 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-08-25 14:30:02 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #62)
> Ok, Rafael, can you look into this? We need traction on this very soon, so
> if you can't, please speak up.

I have some other bugs on my plate (674647, 678558, 673017, 682066), but since this is more important I will try to look at it tonight when I get home.
Comment 64 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-08-25 19:49:53 PDT
The reason the current code is not working is that nsLocalFile::IsExecutable is being passed a file named "foo.jar.part".

The windows code creates a dummy file, that is really ugly, but I will probably piggyback on it for now and clean it up afterwards.
Comment 65 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-08-25 20:07:44 PDT
Created attachment 555937 [details] [diff] [review]
use a dummy file everywhere.
Comment 66 Steven Michaud [:smichaud] (Retired) 2011-08-26 07:50:35 PDT
Comment on attachment 555937 [details] [diff] [review]
use a dummy file everywhere.

I need to test this on multiple platforms before I can approve it.  I'm about to start some tryserver builds.
Comment 67 Steven Michaud [:smichaud] (Retired) 2011-08-26 14:09:50 PDT
Comment on attachment 555937 [details] [diff] [review]
use a dummy file everywhere.

This looks fine to me.

And I got the test results I expected:  Attempts to download my simple
runnable jar testcase (attachment 541078 [details]) now work the same on OS X
and Linux as they do on Windows -- you have no option to run the file,
only to download it.  (And by the way, I made sure to always test with
a clean profile.)

Sorry I missed that nsLocalFile::IsExecutable() was being called with
[xxxx].jar.part :-(

Perhaps the helper app (and local file) code can be cleaned up at some
point.  But this looks like a good interim solution.  

Though (of course) I still don't know much about this code, so there
might be something else	that I missed.
Comment 68 Cameron Kaiser [:spectre] 2011-08-26 14:57:08 PDT
I should have noticed that myself. Sorry. :(
Comment 69 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-08-29 12:52:53 PDT
Pushed to
http://hg.mozilla.org/integration/mozilla-inbound/rev/4a31be812223

Who is responsible for merging this to the old releases? If the answer is "I", is there any docs on the process?
Comment 70 Steven Michaud [:smichaud] (Retired) 2011-08-29 13:01:31 PDT
> Who is responsible for merging this to the old releases?

As the bug's assignee, I suppose you are :-)

> If the answer is "I", is there any docs on the process?

I don't know of any.  But first things first:  Before landing you need to seek approval.  I'd suggest asking for approval at least for aurora and the 1.9.2 branch.

And maybe also for beta.  Dan, Sheila, JP:  What do you think?
Comment 71 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-08-30 05:02:34 PDT
http://hg.mozilla.org/mozilla-central/rev/4a31be812223
Comment 72 Johnny Stenback (:jst, jst@mozilla.com) 2011-09-01 13:19:27 PDT
Can we get this verified now that the patch has landed? Given that we fixed this w/o actually fixing it once, I'd like to be certain that it's fixed for real before we take this on branches etc.
Comment 73 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-09-03 12:20:16 PDT
 > I don't know of any.  But first things first:  Before landing you need to
> seek approval.  I'd suggest asking for approval at least for aurora and the
> 1.9.2 branch.
> 
> And maybe also for beta.  Dan, Sheila, JP:  What do you think?

ping? Is there another process for asking for approval?
Comment 74 Steven Michaud [:smichaud] (Retired) 2011-09-03 16:52:28 PDT
Comment on attachment 555937 [details] [diff] [review]
use a dummy file everywhere.

You ask for approval like so.

But (from comment #72) it sounds like approval won't be granted for any of the branches before someone from QA verifies that the bug is actually fixed.
Comment 75 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-09-06 18:27:05 PDT
(In reply to Johnny Stenback (:jst, jst@mozilla.com) from comment #72)
> Can we get this verified now that the patch has landed? Given that we fixed
> this w/o actually fixing it once, I'd like to be certain that it's fixed for
> real before we take this on branches etc.

Is there anything I need to do (other than the bug flag you added)?
Comment 76 Marcia Knous [:marcia - use ni] 2011-09-08 14:04:35 PDT
I can verify fixed using Steven's simple testcase (https://bugzilla.mozilla.org/attachment.cgi?id=541078) using:

Mozilla/5.0 (Windows NT 5.1; rv:9.0a1) Gecko/20110908 Firefox/9.0a1 and Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0a1) Gecko/20110908 Firefox/9.0a1.

I will verify using Linux in the lab before marking this for final verification.
Comment 77 Marcia Knous [:marcia - use ni] 2011-09-08 14:33:18 PDT
Verified fixed on Linux using Mozilla/5.0 (X11; Linux i686 on x86_64; rv:9.0a1) Gecko/20110908 Firefox/9.0a1. I also checked the latest nightly on Mac 10.7 and it looks good as well. I tested with a clean profile in each instance and confirmed that you only have the option to download the file and not run it (that option is greyed out in Steven's test case)
Comment 78 Johnny Stenback (:jst, jst@mozilla.com) 2011-09-08 14:35:14 PDT
Comment on attachment 555937 [details] [diff] [review]
use a dummy file everywhere.

Approved for aurora and beta, and JP will track that this gets verified etc.
Comment 79 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-09-09 09:51:12 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/7d19319753d9
http://hg.mozilla.org/releases/mozilla-beta/rev/45b03eb031f0
Comment 80 christian 2011-09-09 10:28:34 PDT
Comment on attachment 555937 [details] [diff] [review]
use a dummy file everywhere.

Please land this on releases/mozilla-1.9.2
Comment 81 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-09-09 16:06:02 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/23d85432311d
Comment 82 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-09-09 16:08:32 PDT
QA tracking; will need verification on Aurora, Beta, and 1.9.2 using the testcases attached to this bug.
Comment 83 Johnny Stenback (:jst, jst@mozilla.com) 2011-09-22 13:13:40 PDT
This is fixed on trunk now too, right? If so, can we close this bug by now?
Comment 84 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-09-22 13:19:46 PDT
I think we can close it yes, but I am not sure what is normally the protocol with security bugs.
Comment 85 Vlad [QA] 2011-11-22 07:57:11 PST
I have verified this using the first test from the attachements and the option to open the file is greyed out on:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:9.0) Gecko/20100101 Firefox/9.0 beta 2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0) Gecko/20100101 Firefox/9.0 beta 2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a2) Gecko/20111121 Firefox/10.0a2
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0a1) Gecko/20111121 Firefox/11.0a1

I have also tested this on Firefox 1.9.2 and the "open file" option isn't greyed out on:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.24) Gecko/20111103 Firefox/3.6.24
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.7; en-US; rv:1.9.2.24) Gecko/20111031 Firefox/3.6.24

This is intended? Can the resolution be changed to Verified?
Comment 86 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-22 08:26:29 PST
I believe so, Vlad.
Comment 87 Steven Michaud [:smichaud] (Retired) 2011-11-22 08:38:45 PST
Testing with my "simple runnable jar file" testcase a clean profile in FF 3.6.24 (on OS X 10.7.2), this bug *isn't* fixed -- FF 3.6.24 still defaults to running the jar file in JarLauncher.

Sigh :-(

Also, this bug is only fixed in FF 7.0 and up -- not in FF 5.0.1 or 6.0.2.
Comment 88 Steven Michaud [:smichaud] (Retired) 2011-11-22 08:40:59 PST
> Also, this bug is only fixed in FF 7.0 and up -- not in FF 5.0.1 or 6.0.2.

Oops, misread our confusing user-agent strings.  Now I see that Vlad only tested on 9.0 beta2 and up.
Comment 89 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-22 08:44:06 PST
(In reply to Steven Michaud from comment #87)
> Testing with my "simple runnable jar file" testcase a clean profile in FF
> 3.6.24 (on OS X 10.7.2), this bug *isn't* fixed -- FF 3.6.24 still defaults
> to running the jar file in JarLauncher.
Should this bug be cloned so the patch can be backported to 1.9.2?

> Also, this bug is only fixed in FF 7.0 and up -- not in FF 5.0.1 or 6.0.2.
Firefox 9.0 is the point-release for all previous versions.
Comment 90 Steven Michaud [:smichaud] (Retired) 2011-11-22 08:47:03 PST
> Should this bug be cloned so the patch can be backported to 1.9.2?

It *has* been backported to the 1.9.2 branch (see comment #81).  It just doesn't work there, apparently.

I suppose it makes sense to open a new bug on that, though.

Note You need to log in before you can comment on or make changes to this bug.