154346 - Mozilla looks like it loads and opens executables automatically (though it does not)

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect)

Description

[Andrei Leo]

When clicking on .exe file link Mozilla downloads and opens file automatically.
This migth be very dangerous especially when download prefrences are set to
"don't open anything".

Reproducable:Always
Steps to reproduce:
1. Go to http://www.mozilla.org
2. Click on Download Win 

Actual results: 
Mozilla will start downloading file automatically and will try to run it after
downloading finished.

Expected:
Download Dialog with options to Save or Run this file.

MZ 2002062508, W2K

Comment 1

[Robert Wall]

Under Edit->Preferences->Navigator->Helper Applications, if you press Reset,
does this cause Mozilla to ask you if you want to download/open exe files? 
(Note: pressing reset will cause Mozilla to confirm all types, even those you
may have specified to download automatically). 

Comment 2

[Boris Zbarsky [:bzbarsky]]

Could you attach your mimeTypes.rdf and prefs.js files here?  This should never
happen, period.

Comment 3

[Andrei Leo]

Edit->Preferences->Navigator->Helper Applications, Reset solved this situation.

It happened...
...and, actually, I found similar bug 149782. It says also that there was no
dialog window when saving exe.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Arrrrgh.  I realize that it happened.  But it should not, so we should find out
why it happened and fix it... unfortunately you have cleared the setup that
could trigger the problem, which makes diagnosing it nearly impossible.

Can you possibly reproduce the problem again?

Comment 5

[Andrei Leo]

The reason of this behaviour was probably this row in prefs.js

user_pref("browser.helperApps.neverAsk.openFile", "application%2Foctet-stream");

This row appears in prefs.js after I clear checkbox "Always ask before opening
this type of file" in download dialog box for .ICA file ("Citrix ICA Connection"
[application/octet-stream]). By doing so I expected no to be asked about this
only particular ( .ica ) type of file. ( I want it to be opened automatically by
Citrix plugin )

It's up to you to decide is it a bug or not.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yes, that would be a bug.  We're supposed to ignore that pref for .exe files,
and in fact we have code to do that.

I don't have a windows system on hand, so could someone who actually runs
windows test this please?  Confirming to get on the radar.

Comment 7

[Michael Lefevre]

using 1.1alpha here on win2k, and I can't reproduce this.

I tried making an .ica file (actually some other random binary) in my webspace,
but downloading it causes mozilla to name it blah.ica.exe, and I don't get the
chance to turn off the "always ask" box.  tried adding the mentioned pref
manually, but mozilla still just brings up the save box.

will try again this evening with the latest trunk...

Comment 8

[gabriel]

Renaming is due to bug 120327. I don't know if that is relevant or not.

Comment 9

[Andrew Hagen]

I added that pref from comment 5 to user.js. I started Mozilla 1.1 alpha Build
ID: 2002062404, Windows 98, and loaded the profile that had the user.js file. I
went to http://www.mozilla.org and clicked on the Mozilla 1.0 installer for
Windows. The download began without any prompting. When it finished, an error
message came up. The error message is the same as in bug 149782. I clicked on
"ok." No executable ran, and no file was saved.

In bug 149782, the error message has a screenshot,
http://bugzilla.mozilla.org/attachment.cgi?id=86730&action=view .

Comment 10

[Mitchell Stoltz (not reading bugmail)]

I am marking this bug Security-Sensitive until we fix it or determine that it
isn't serious. Only those CC'd on the bug, plus a select group of trusted
people, will be able to view the bug. If anyone objects, just uncheck the
Security-Sensitive box.

Comment 11

[Daniel Veditz [:dveditz]]

Nominating for a look-see. this is very not good.

Comment 12

[Jaime Rodriguez, Jr.]

Removing adt1.0.1 because there is nothing to approve for checkin at this time.
Adding nsbeta1+/ADT1 as this is a serious security issue. pls add the adt1.0.1
nomination back, when there is a patch that has landed on the trunk, and been
verified. thanks!

Comment 13

[Blake Ross]

This has nothing to do with download manager as far as I can tell.

Comment 14

[Michael Lefevre]

someone beat me to it, but FWIW, I tried this again with today's trunk on
win2000, and comment #9 describes what happened (the error message).  if this is
reproducible, then I guess it needs some unknown tinkering with the mime types
as well as that pref.

Comment 15

[scottputterman]

Bill, any progress on this?

Comment 16

[Bill Law]

Good news, bad news:

The good: I don't think we ever actually *run* the .exe file.  The initial
reports used the phrase "try to run" and the other reports explicitly describe
the error that would result if we did try to run the .exe.  This is caught by
the code here:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/win/nsOSHelperAppService.cpp#76


Thank goodness we (Boris and I) decided to put that failsafe mechanism in there.

The bad: Users have trouble downloading executables: they don't get the chance
to say they want to save them to disk.  Plus, they get puzzled and/or scared by
the alert that eventually appears.

I don't think that's a stop-ship.  The user only gets in this predicament if
they  are silly enough to say they always want to open content of type
application/octet-stream (actually, there may be more rare occurrences involving
other content types in conjunction with misconfigured or malicious web sites).

There is a remedy: Go to The Navigator/Helper Applications pref panel and press
the "Reset" button.  This could perhaps be covered in release notes and that
would be sufficient.

A relatively simple code fix would be to test explicitly for executable files
where we check the "neverAsk" pref (see
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1156).
 Unfortunately, I have just had 3 or 4 of these last-minute urgent,
death-defying problems dumped on me today so I'd rather pawn this one off on
somebody else.  Boris?  Please?

Caveat: All this is based on a cursory examination of the code.  I could be
wrong (let's put it this way: I have more confidence that what I'm stating is
true than I have confidence that there's not worse problems still lurking in the
maze of twisted logic that comprises this particular part of the browser :-).

Comment 17

[Bill Law]

Boris has agreed to take on the implementation of the fix I sketched out 
above.  Thanks, Boris.  Give me a holler when you've got the fix ready for 
review.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: Hack to improve appearances

This should do it... (haven't tested yet, but about to).

Comment 19

[Boris Zbarsky [:bzbarsky]]

jst has tested this and it's good.  So some reviews would be nice.  Nominating
for branch; this is a serious user-perception issue, as you can see by the
comments in this bug.  In addition to being unable to save the file easily,
users get the impression that a security hole exists.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: tweak that test slightly

Comment 21

[Bill Law]

Comment on attachment 89637 [details] [diff] [review]
tweak that test slightly

r=law

Looks good.  I'll try to get somebody here to sr.

Comment 22

[Bill Law]

Adding jag to cc: list so he can sr.

Comment 23

[jag (Peter Annema)]

Comment on attachment 89637 [details] [diff] [review]
tweak that test slightly

sr=jag

Comment 24

[Boris Zbarsky [:bzbarsky]]

Fix checked in on the trunk.  Mitch, please remove the security-sensitive
designation, since there is in fact no security problem here.

Comment 25

[chris hofmann]

Comment on attachment 89637 [details] [diff] [review]
tweak that test slightly

a=chofmann for 1.0.1 add fixed1.0.1 to the keywords when  checked in.

Comment 26

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to th 1.0. branch. pls check
this in asap, then replace "mozilla1.0.1" with the keyword "fixed1.0.1". thanks!

Comment 27

[scottputterman]

chofmann gave drivers approval in comment 25, so adding mozilla1.0.1+.  Please
check this in today.

Comment 28

[sairuh (rarely reading bugmail)]

tested this with both mozilla and commercial trunk bits (2002.07.01.08) on win2k.

results:

a. *without* the pref in comment 5 being set (to avoid being prompted), i do get
prompted --ie, the file picker dlg to save appears. this is as expected, with
both the commercial and mozilla builds. (control test, really.)

b. results for the commercial and mozilla trunk builds with the pref in comment
5 ON (to ignore): unlike (a), the the download manager appears, the file starts
to download...the an error dlg appears saying "[temp-path]\[file.exe] could not
be opened, because an unknown error occurred. Sorry about that. Try saving to
disk first and then opening the file." the "progress" result in the dl mgr is
"failed".

is (b) expected behavior? the file isn't launched, at least...

Comment 29

[sairuh (rarely reading bugmail)]

s/launched/opened

if this behavior is expected, then i'll go ahead and verify it for the trunk.

Comment 30

[Boris Zbarsky [:bzbarsky]]

*** Bug 149782 has been marked as a duplicate of this bug. ***

Comment 31

[Boris Zbarsky [:bzbarsky]]

(b) is exactly the behavior we are trying to prevent.  I will attempt to locate
a Windows machine and retest the patch again; in the meantime the exact steps
you used in step (b) would be very helpful.

I'd like to hold off landing this on the branch till it's actually verified that
the patch fixes the problem....

Comment 32

[Michael Lefevre]

I'm not quite clear what the patch is aiming to change, but the behaviour you've
just described (comment 28) sounds the same as the pre-patch behaviour,
described in comment 9, so I don't see that you've verified any change.

(apologies if I'm on the wrong track here...)

Comment 33

[Boris Zbarsky [:bzbarsky]]

This is what I get for testing and then making changes in response to review
comments... ;)

Comment 34

[Boris Zbarsky [:bzbarsky]]

Attachment: This should be much better; going to test it now

Comment 35

[Boris Zbarsky [:bzbarsky]]

Testing shows that (unlike attachment 89637 [details] [diff] [review]) this works and (unlike attachment
89622 [details] [diff] [review]) it does not break automatic execution of other content.  reviews, please?

Comment 36

[Bill Law]

Comment on attachment 89838 [details] [diff] [review]
This should be much better; going to test it now

r=law (and this time I mean it :-)

Comment 37

[jag (Peter Annema)]

Comment on attachment 89838 [details] [diff] [review]
This should be much better; going to test it now

sr=jag

Comment 38

[Boris Zbarsky [:bzbarsky]]

Correct fix checked in, trunk and branch.  When verifying, please make sure to
test the following:

1)  When the "never ask" pref is _not_ set, .exe files should cause a filepicker
    to be put up directly.
2)  When the "never ask" pref _is_ set, .exe files should still cause a filepicker
3)  If the "never ask" pref is set to "application%2Fpdf" then PDF files launch in
    acrobat viewer without prompting the user (this may require disabling the
    acrobat plugin or testing with another MIME type if you have a helper set up
    with it and all that).

Comment 39

[sairuh (rarely reading bugmail)]

boris, here are the steps i had taken for case (b)

1. in the prefs.js file for the profile i'm using, i have set the following:

user_pref("browser.helperApps.neverAsk.openFile", "application%2Foctet-stream");

2. i start the brower and go to http://mozilla.org/

3. i click either of these links:

* under the "Mozilla 1.0" download section, clicked the "Win (9.8 MB)" link.

* under the "Mozilla 1.1 Alpha" download section, clicked the "Win (10.9 MB)" link.

Comment 40

[Boris Zbarsky [:bzbarsky]]

Yeah, those are it.  The patch was just not correct; thanks for catching it!

Comment 41

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.07.02.08 branch comm bits on win2k.

tested using boris' three cases in comment 38 --i used the links in comment 39
for the first two cases, and a couple of pdf files (after disabling the acrobat
plugin to make the acrobat viewer app launch instead) on my server for the third
case. looks good!