Closed Bug 116938 Opened 23 years ago Closed 22 years ago

tries to save .exe file rather than play it

Categories

(Core Graveyard :: File Handling, defect, P1)

x86
Windows 2000
defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.0

People

(Reporter: jamesrome, Assigned: law)

References

()

Details

(Keywords: privacy, Whiteboard: [adt1])

Attachments

(3 files, 4 obsolete files)

Links such as http://www.content.loudeye.com/scripts/hurlPNM.exe?/~ttt-466568/0334758_0102_07_0002.ra appear on the http://www.bmgmusicservice.com pages to allow customers to play audio clips. Mozilla trys to save the URL as hurlPNM.exe rather than to execute it on the server and run real player.
Over to Networking.
Assignee: asa → neeti
Component: Browser-General → Networking
QA Contact: doronr → benc
I'm getting "Content-Type: audio/x-pn-realaudio" in the HTTP header, so apparently Mozilla is ignoring the MIME type in favor of the file extension?
We only do that if we can't find a handler for that mime type... Does setting up a helper in helper apps for audio/x-pn-realaudio help? Also, the server is doing something odd. I can get the data from it with telnet but not with wget. Can someone open a command prompt, set NSPR_LOG_MODULES to "nsHttp:5" then start mozilla and record the log output to a file while opening the above URL, then attach the resulting log to this bug?
ordinary Real URLs work just fine for me. Such as http://sunsite.utk.edu:8080/ramgen/encoder/wuot.rm It asks me if I want to open it with Real Player.
Win2K SP2 buildID 2002020206 Clicking on the link tries to save the file (hurlPNm.exe), adding a file type in the Helper Applications/file extensions with values: RA audio/x-pn-realaudio, open with realplayer, clicking on the link opens dialog asking me to choose between opening with [realplayer] or saving, (works so far), however, it still says it's doing something with the .exe file, and I don't have realplayer installed. Setting the mimetype made moz open the correct dialog however.
OK. This worksforme when I have realplayer defined in helper apps. I bet what happens here is the following: 1) We try to find what to do with audio/x-pn-realaudio files. Nothing is found in prefs or the system setup 2) We look at the extension in the url. It's ".exe". We look for something to do with an exe file. The only option is "save". So the problem is in the lookup in step #1 failing. James, could you attach your mimeTypes.rdf file to this bug if you're still seeing the problem?
The site changed the URLs so it no longer has that format.
Attached file mimetypes.rdf file as requested (deleted) —
Actually, that URL now asks me if I want to open it with Real Player, I say yes, and nothing happens.
Attachment #69393 - Attachment mime type: text/rdf → text/plain
OK. No mentions of "audio/x-pn-realaudio" in the file... James, what build are you using? I get prompted to run realplayer and then it runs and the music plays...
When I try the URL now, hurlPNM-4.exe gets put in /tmp, even though I said open with real player. No .rm file is put there.
Whow, this is ugly. I have RealPlayer installed and working ok as a helper app. I've set it to always open with RP without confirming. What Mozilla (build 20020214, Win2K) really did was to save the ra file as hurlPNM.exe to the temp folder and EXECUTE it (causing the Win16 subsystem to crash as it apparently was not a valid exe). If I interpreted the result corrently, this is clearly a huge security hole allowing to masquerade an (possibly malicious) executable as .ra file. Confirming and raising priority.
Severity: normal → critical
Status: UNCONFIRMED → NEW
Ever confirmed: true
...meant to say changing severity, not priority. Sorry for the spam.
file handling... I think I know what's up here.
Assignee: neeti → law
Component: Networking → File Handling
QA Contact: benc → sairuh
Attached patch Proposed patch (obsolete) (deleted) — Splinter Review
Keywords: patch, review
I'm glad I finally uncovered a big security hole :-)
I should note that the fact that any file with a .exe extension is treated as an executable in these circumstances by the system even though Mozilla _explicitly_ runs the realplayer app on that file is odd beyond belief.... :)
Summary: trys to save .exe file rather than play it → tries to save .exe file rather than play it
I'm afraid I still don't understand exactly what's going on here. Boris, can you spell it out a little more explicitly, please? Where exactly is the security hole? It strikes me that the fix here is too extreme. I'm not sure why we shouldn't be respecting the content-disposition header, or the actual name of the file on the server, just because the user has set up their helper-app pref entry incorrectly. Sorry if I'm being dense.
Bill, the problem here is that we copy the file to the mSuggestedFileName before running the helper on it (a somewhat recent change, right?). So here is the problematic sequence of events: 1) The user does not set up a helper for audio/x-pn-realaudio, but there is a registry entry for it. 2) At some point the user selects "never ask me for this type" on an audio/x-pn-realaudio file (a perfectly reasonable thing to do). 3) A server sends data as type audio/x-pn-realaudio, with a content-disposition filename of "foo.exe" (or the filename in the URL is foo.exe, as here, in which case the server is not really being malicious....) 4) OnStartRequest never opens the helper app dialog (since we have "never ask" set), hence we never do the isExecutable() check from the helper app dialog. Thus we proceed to LaunchWithApplication() and try to launch the file via the system shell. And since we do this after copying to mSuggestedFilename it ends up being run as an executable. Needless to say, a malicious server could use Content-Disposition and sending executable data as some random (eg audio/x-pn-realaudio) type to silently run code on the user's machine. I agree that my fix is a little extreme... Other options would be: 1) Add an isExecutable() check in the "never ask" branch of OnStartRequest, just like we have in the helper app dialog. For that matter, we could do the check in OnStartRequest before the "do we ask?" check, and thus avoid it completely in the helper app dialog. 2) Only munge the filename if we're planning to "Open with application", not if we plan to "save as" 3) Do #1 and do the extension-munging for a filename we get from the URL as per my existing patch. Of these three, #3 is the only one that will really solve the original issue in this bug. What happened to James initially was that he did not have "never ask" checked and isExecutable() tested true in the helper app dialog. So we forced to "Save As" as a security measure. Going to attach a patch attempting to do #3.
Attached patch Patch v1.1 (obsolete) (deleted) — Splinter Review
Implement my suggestion #3. This needs a security check-over (Mitch?).
Attachment #69867 - Attachment is obsolete: true
As a note, the fix I just attached will _still_ sometimes munge the filename if said filename comes from a url. It won't do it often, imo, and it'll do it in a manner consistent with "save link as" (except slightly more accurately, since it already has the real type the server sends).
>the problem here is that we copy the file to the mSuggestedFileName before >running the helper on it (a somewhat recent change, right?). There was a change to copy/rename the file after the download was complete, yes. I think we always set the extension, however. But that may not have been subject to content-dispostion header influence. Also, I'm not sure exactly what precedence was given to the url's extension vs. the helper app entry's. Note that it is this uncertainty that makes me cautious about leaping too quickly on some new fix. Anyway, thank you for explaining the problem. I was uncertain whether we would ever do a ShellExecute against a file for which there was a helper app entry. Perhaps the hole is in the case where there's a pseudo-helper app entry, as is the case when the user says "don't ask me" for a given mime type. Is that the only hole? If they say "don't ask me" in a *real* helper app entry, then that entry will have to be either save-to-disk, or, specify a real application (which we will launch). If that is the thing that needs fixing, then that could be handled at the point where we manufacture the "pseudo" helper app entry from the don't-ask-me pref list (by checking for IsExecutable()). Anyway, we'll see how closely your latest patch matches that speculation. Oh, one other thing. I am worried about the case where the user creates a helper app entry for some random type, like application/octet-stream. If this entry has no extension list, then what happens if there's a .zip file with type application/octet-stream? I thought your previous patch would preclude suggesting .zip as a file extension when the user saved, and I think that would be wrong. I think we should consider this case when examining the other solutions. OK, one *more* thing: I'd prefer to see the "policy" implemented at as high a level as possible (higher meaning further from the guts of the uriloader). That way, different mozilla-based browsers could institute different policies more easily.
> makes me cautious about leaping too quickly on some new fix Agreed. Let's do this right once and for all. > Is that the only hole? I _think_ so, but I don't claim to understand the Windows nsOSHelperAppHandler that well... > If this entry has no extension list In that case mTempFileExtension is empty, and so we just keep the "original" filename.... The issues start if the user has a helper app setting for application/octet-stream listing "foo" as an extension. In that case we would append ".foo" to the ".zip" file. I'm not sure how acceptable that would be. So the two things we're trying to fix are: 1) "Don't ask me" helpers gotten from the system never get an isExecutable() check 2) A url ending in .exe will preclude running helpers on content coming from that url because we use the URL filename as the helper filename. Thus the user just gets a save dialog.
>I _think_ so, but I don't claim to understand the Windows nsOSHelperAppHandler >that well... Makes my head spin, too (although I'm usually puzzling over it at 2:00am so maybe it's just me). >The issues start if the user has a helper app setting for >application/octet-stream listing "foo" as an extension. In that case we would >append ".foo" to the ".zip" file. I'm not sure how acceptable that would be. Not very, I suspect. The thing is that this twiddling of the extension is just a means to an end (the end being not doing ShellExecute on an .exe file). We can block that by other means, I think. >1) "Don't ask me" helpers gotten from the system never get an isExecutable() > check Right. And there's really two sides of this: a. We need to make sure that executable files don't get ShellExecute-d. b. We need to make sure that non-executable files get an extension based on the helper app settings (but only if they will be ShellExecute-d). If we don't do this, then it is unlikely that the system will launch the proper helper app. >2) A url ending in .exe will preclude running helpers on content coming from > that url because we use the URL filename as the helper filename. Thus the > user just gets a save dialog. Unless they say "don't ask me," right? Because in that case, we go straight down the "launch with application" path and if there's really a helper app, then we run it against the temp file (and it doesn't matter what the temp file is named). It seems like the problems occur only when we run ShellExecute. But that might be a rash assumption. But if it were the case, then maybe we could just do the IsExecutable() check at the point where we're about to do the ShellExecute. Provided we can deal with the situation properly at that point in time, then that might cover all the IsExecutable() issues. BTW, we always do ShellExecute via the "Launch" button on the progress dialog, so that needs an IsExecutable check, regardless.
> twiddling of the extension is just a means to an end (the end being not doing > ShellExecute on an .exe file) Actually, no. The end is, in the orifinal incarnation of this bug, to be able to listen to realaudio content served from a url which ends in ".exe". Agreed on points a. and b. in your comment. :) Moving the IsExecutable() check to right before ShellExecute sounds like a very good idea. In fact, if we want to be nice to embeddors we could add a bool argument to nsPIExternalAppLauncher::launchAppWithTempFile that could be set to true to bypass the security check (if embeddors want to, eg, enable running of executables directly). Would that work?
Making the security check an option on the nsPIHelperAppLauncher method is a good enhancement. So how do we handle the situation when about the ShellExecute the .exe file? We need to make sure an appropriate error response filters back to a point where we can give the user the "save to disk" option.
Have any fixes for this been put into the nightly builds? Bug 127322 I filed on the 2/22 build which now forgets mozillacompose.xul and tries to download it. Hence you can't send mail except for the first time.
Nope. If there were changes that tried to fix this bug there would be reviews and such in here....
*** Bug 127536 has been marked as a duplicate of this bug. ***
Nominating, due to security ramifications.
Keywords: nsbeta1
Setting target milestone. Boris, are you willing to be assigned bugs such as this that you're working on? Otherwise, I'm going to have to go look for some victim on which to offload some of my bugs (or they will get pushed out).
Target Milestone: --- → mozilla1.0
Comment on attachment 70087 [details] [diff] [review] Patch v1.1 I'm not actively working on this one right now... For one thing, I have no windows system to test on, which makes changes to windows-only code hard to develop/debug... I can try to do it if there is no one else, but it may take me some time...
Attachment #70087 - Attachment is obsolete: true
Attachment #70087 - Flags: needs-work+
Attachment #70088 - Attachment is obsolete: true
Attachment #70088 - Flags: needs-work+
I'll take it and trust I can consult with you as needed; thanks.
Yeah. I'm certainly up for that. :)
nsbeta1+ per ADT triage team
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt1]
I still can't get Real links to play. Columbia House has: <!--Switch Row Colors Note : See if we can move to method based variable so we can remove one copy--> <tr> <td bgcolor="#cccccc" width="302">Frederic Chopin (1810-49)</td> <!-- CHECKING FOR WINDOWS MEDIA --> <td bgcolor="#cccccc" align="center"><a href="http://www.columbiahouse.com/m2/rams/asfhier/35/01/350116_01_00_2.asx" onMouseDown="CMPageView(this)" ><img src="//a248.e.akamai.net/f/1596/1167/1d/images.columbiahouse.com/ch/images/se2/ch_music/master/listen.gif" width="22" height="12" alt="Frederic Chopin (1810-49)" border="0"></a></td> <!-- CHECKING FOR REAL AUDIO --> <td bgcolor="#cccccc" align="center"><a href="http://www.columbiahouse.com/m2/rams/rm5hier/35/01/350116_01_00_5.ram" onMouseDown="CMPageView(this)" ><img src="//a248.e.akamai.net/f/1596/1167/1d/images.columbiahouse.com/ch/images/se2/ch_music/master/listen.gif" width="22" height="12" alt="Frederic Chopin (1810-49)" border="0"></a></td> <!-- CHECKING FOR REAL AUDIO G2 --> <td bgcolor="#cccccc" align="center"><a href="http://www.columbiahouse.com/m2/rams/rmhier/35/01/350116_01_00.ram" onMouseDown="CMPageView(this)" ><img src="//a248.e.akamai.net/f/1596/1167/1d/images.columbiahouse.com/ch/images/se2/ch_music/master/listen.gif" width="22" height="12" alt="Frederic Chopin (1810-49)" border="0"></a></td> </tr> The .ram links pop up a window. I select "open with real player," and then real comes up with an invalid metafile message, trying to play file://C:/TEMP/350116_01_04_5.ram
James, Your last report seems to be a separate problem. It looks like we're downloading the content with the correct extension (.ram) and launching real- player, but the data is bad. That could be a problem on the server, or something else. Can you save one of those .ram files (using "save link as") and try opening realplayer on it directly?
Hmmm. The "file" contains rstp://nyra2.columbiahouse.com/g2rm/35/01/350116/350116_01_00.RM and if I put the rstp URL in a mozilla URL I get "r is not a registered protocol" If I put it into RealPlayer, I get "A general error has occurred"
*** Bug 136328 has been marked as a duplicate of this bug. ***
Regarding the bug that was just duped to this one, I found a windows machine to test on. I installed realone on it. Tried my test URL (http://www.kryptolus.com/t.exe) and the executable was automatically executed.
So it is, (http://www.kryptolus.com/t.exe) first opens RealPlayer and than is executed automatically with Mozilla/5.0 (Windows; U; Win98; de-AT; rv:0.9.9+) Gecko/20020403(03)
I'm going to extract a subset of the big patch I've got for bug 86640 to plug this security hole. That will be a very small-scale and low-risk patch.
Whiteboard: [adt1] → [adt1] ETA 4/16
Attached patch revised patch (obsolete) (deleted) — Splinter Review
This patch has two parts: 1. Ensure mSuggestedFileName always has mTempFileExtension This is accomplished by adding a new utility method to nsExternalAppHandler called EnsureSuggestedFileNameExtension. This function compares the file extension in mSuggestedFileName to that in mTempFileExtension. The latter is always tied to the content type (see code in ::DoContent). If that extension differs from what's in mSuggestedFileName, then it is tacked onto the end of mSuggestedFileName. This function is called from two places: SetupTempFile where mSuggestedFileName is initialized, and ExtractSuggestedFileFromChannel where it is reset based on the Content-Disposition response header. Thus, http://www.kryptolus.com/t.exe produces "t.exe.ram" as suggested file name (and the name of the file passed to ::ShellExecute). Something like "http://www.foobar.com/foobar.ram" where the site does something evil like return Content-Disposition: attachment; filename="foobar.exe", will produce a suggested file name of "foobar.exe.ram". 2) win/nsOSExternalAppService.cpp is changed so that it opens temp files indirectly via nsLocalFileWin::Launch, after checking that the file is not executable (if executable, it returns an error). Item 1) above makes it so this code can't be reached, but I put the check in just to be safe (there may be some deviant path through nsExternalHelperAppService that could get you here). It also buys a bit of functionality in that there is code in nsLocalFileWin::Launch that will display the Win32 application-picker if there is no application currently associated with the file extension. Samir or Boris, can you review, please? Who's up for super-review?
Comment on attachment 79377 [details] [diff] [review] revised patch I noticed that the original ShellExecute call in nsOSExternalAppService passes "open" as the 2nd argument. nsILocalFileWin::Launch passes NULL for this argument. I seem to remember it being a bug when we weren't passing "open" in the external nsOSExternalAppService. Are we opening ourselves up to a bug by changing that behavior? The code looks great to me. Couple testing comments: 1) Can you try opening some mail attachments the require helper apps like MS Word, Real Audio and Acrobat. 2) Can you try opening some content that uses a content disposition header for getting the file name. Just wanted to make sure we still behave correctly when determining the file name in this scenario.
Comment on attachment 79377 [details] [diff] [review] revised patch This will make our suggested filename suck in the "save" case.... Could we possibly call EnsureSuggestedFileNameExtension right before the MoveFile/Launch combination? At that point we know for sure we're launching it....
re: comment 45 I don't know about the NULL vs. "open" on ::ShellExecute. I'll investigate that. I'm not sure why it would need to be different here versus when opened via the Launch button on the progress dialog or dialog manager (the other place where nsLocalFileWin::Launch is called). It may be that "open" is *different* than NULL (opens versus "do the default"), but I'm not sure we should do different things, so maybe nsLocalFileWin::Launch should be fixed. I've tested the Content-Disposition case. We still respect the filename specified there, and modify it if its extension is bogus. re: comment 46 Suck? How so? We suggest with an extension that makes sense (meaning: if the user saves with the name we suggest, they get a file whose extension that actually indicates the type of content in the file). In the canonical case (t.exe served up as audio/x-pn-realaudio), I don't think we want to encourase the user to save that as a file with a .exe extension.
Whiteboard: [adt1] ETA 4/16 → [adt1] ETA 4/17
Is this in a nightly build yet so I can test?
> Suck? How so? For example it'll append ".txt" to everything that's text/plain and does not have a ".txt" extension (this is pretty gratuitous). The contentAreaUtils.js code already has that problem..... For another example, it'll append extensions to the server-selected name on Mac and Linux and other OSes where the extension is not what determines the file type. My suggestion would be to: 1) Always twiddle the extension in the "execute" case 2) Only twiddle the extension in the "save" case if a) We're on Win32 or OS/2 or b) We're not, and the type is not text/plain and the filename came from the URL, not the content-disposition header I'd be happy with just doing 1) and 2a) for now... (and I'll probably do 2b at some point, but that's not a security issue, whereas 1 and 2a are). > Is this in a nightly build yet so I can test? No. There'll be a comment about it getting checked in before it is.
With the 41603 build http://www.content.loudeye.com/scripts/hurlPNM.exe?~ttt-466568/0337466_0107_07_0002.ra now opens a dos box, runs something, but never gets the link to real player.
>For example it'll append ".txt" to everything that's text/plain and does not >have a ".txt" extension (this is pretty gratuitous). The contentAreaUtils.js >code already has that problem..... Well, everything that's text/plain will just open in the browser :-). But your point is still valid, to some extent. Given something like "http://www.music.com/MyFavoriteSong" maybe saving it as "MyFavoriteSong" is better than "MyFavoriteSong.ram" (on Mac and Linux). I can't imagine a Mac or Linux user who would want an audio/x-pn-realaudio file to be named "t.exe," however. This is why I hate working on this code (and at least I'm getting paid to do it :-). >1) Always twiddle the extension in the "execute" case By "execute" I presume you mean "open with application" (from the helper app dialog, versus save-to-disk). But if, as you say, "the extension is not what determines the file type," then why does it matter what extension we put on the file (on Mac and Linux and other such OSes) in that case? Also, is it really the case that on Linux that the extension does not determine the filetype? There's no MIME type in the file system, so after it's saved, the extension is the only clue the system has. Don't the desktops use the extension to determine which application to open (when you double-click the file on the desktop)? I would think we would need to help the user exploit that (by offering to put the "proper" extension on the file when we offer to save it).
> But if, as you say, "the extension is not what determines the file type," then > why does it matter what extension we put on the file Because there are unfortunately many applications (staroffice, Mozilla, etc) that care about the extension. So in the "open with application" case it's better to be safe and add the extension. > There's no MIME type in the file system, so after it's saved, the > extension is the only clue the system has. Well, nicely behaved things use the 'file' utility, which looks at magic numbers in the file (and current distributions have magic-number-to-mime mapping files). But yes, desktops do tend to key off extensions.... Which is why I suggest that we _do_ add the extension if it came from the URL (since the URL can have nothing to do with the file type). I suggest we keep content-disposition filenames on Linux/Mac when saving, however (you're right that text/plain is not an issue in this code). So my revised summary is: 1) On Windows and OS/2 always fix up the extension 2) Otherwise, fix it up if a) We're going to "open with application" or b) The filename came from the URL. Does that make sense? > I can't imagine a Mac or Linux user who would want an audio/x-pn-realaudio > file to be named "t.exe," however. True. That offers an alternative, which is to fix up the extension on those OSes in the save case only if there is already an extension on the file.... I think the other method is better, but that's just me. I'd be fine with either one. > This is why I hate working on this code agreed. :)
> Which is why I suggest that we _do_ add the extension if it came from the URL I meant "Which is why I suggest that we _do_ add the extension if the filename came from the URL"
>Because there are unfortunately many applications (staroffice, Mozilla, etc) >that care about the extension. So in the "open with application" case it's >better to be safe and add the extension. Seems odd that these "many applications" would suddenly *stop* caring after the file is saved. But I'll concede you this one since Linux users are easy enough to please, regardless. Now what about MacOS X? That seems real file-extension sensitive so maybe we need to try to set the extension properly there, too (although i don't know if the Internet Config code on that platform even supplies us with file extension hints). I'll work on code that implements your strategy and post it here tomorrow.
Whiteboard: [adt1] ETA 4/17 → [adt1] ETA 4/18
BTW, I tried opening a mail attachment and it worked fine. I'm not sure how I can construct a mail attachment where the mime type and file extension are at odds, though. Are there scenarios like that which I should try, and if so, how?
> Now what about MacOS X? Frankly, I don't know. I do recall that it keys off extension, though, so we may want to make that do the same thing as OS/2 and Windows... > I'm not sure how I can construct a mail attachment where the mime type and file > extension are at odds, though. Just sent you a PDF attached as application/pdf with a ".doc" extension on the filename.
This is what you get when opening an attachment of type application/pdf when it is named *.doc. Note that I had to remove my Acrobat plugin to get that (otherwise, it opened in the mail window). Note the file name on the dialog title bar. That's mSuggestedFileName, which has the .pdf extension in this case.
Attached patch final revision (deleted) — Splinter Review
I started to implement the Boris Strategy. But it really complicates the code. Instead of one code path, we have about 20 (save vs. open, filename from url vs. from content disposition header, then different logic for every platform). We (I, after consultation with some other respected engineers who were around) decided that it wasn't worth it. 1. On Mac classic, it doesn't much matter because everybody is going to MacOS X anyway. 2. When saving a file, one very likely still wants the extension set right since that influences what happens when the file is opened on the system desktop. 3. For every case where the extension we arrive at is worse than the one from the URL (or content disposition header), there will probably be a thousand where it is better (i.e., in the vast majority of cases where they are even different, the default will be .cgi or .dll). So this patch implements the same basic strategy. I've made two concessions by changing the name of the function to make it more generic ("EnsureSuggestedFileName" instead of "EnsureSuggestedFileNameExtension") and made the function virtual so it will be easier for platforms to override the default behavior. Will you sign off on this, Boris? We can revisit the issue again when I try to land the fixes for bug 86640.
Attachment #79377 - Attachment is obsolete: true
Comment on attachment 79672 [details] [diff] [review] final revision r=bzbarsky Yeah, I can see that the other version got complicated... I'm fine with getting this in for now (that gives us about the same behavior as "save page/link as", actually). I'll likely end up revisiting this in the context of fixing the pure "save as" bugs on the extension-adding behavior... at that point, we'll likely want to make this function scriptable so all the icky filename logic can live in one place.... But for now let's just fix the security bug. :)
Attachment #79672 - Flags: review+
Need sr= from Scott. I could find no documentation that would lead me to believe there is any difference between specifying NULL vs. "open" on the ::ShellExecute. I haven't seen any bugs relating to the Launch button on the progress dialog, either. So I don't think there's a problem there and if there is, we need to fix it over in nsLocalFileWin::Launch.
Comment on attachment 79672 [details] [diff] [review] final revision Ok, thanks for looking into the "open" stuff Bill. sr=mscott This patch will also fix our compatibility with: www.mlb.com when you try to listen to a live baseball game. The temp file isn't getting the right extension so real one isn't launching. Your patch fixes that. Thanks!
Attachment #79672 - Flags: superreview+
This is *still* not fixed in RC1 -- this bug is a huge security hole which I would consider a must-fix for any milestone, and certainly a release candidate. Please fix this in RC2!!
adding adt1.0.0 nomination
Keywords: adt1.0.0
adding adt1.0.0+. Please check into the branch as soon as possible and add the fixed1.0.0 keyword.
Keywords: adt1.0.0adt1.0.0+
Comment on attachment 79672 [details] [diff] [review] final revision a=shaver for 1.0 branch.
Attachment #79672 - Flags: approval+
Blocks: 133751
Blocks: 139500
Bill, FWIW, there is a difference between NULL and "open" on ::ShellExecute. Specifically, NULL will pick the 'default' action. From http://msdn.microsoft.com/library/default.asp?url=/library/en- us/shellcc/platform/Shell/reference/functions/shellexecute.asp [when using NULL as the parameter] For systems prior to Microsoft® Windows® 2000, the default verb is used if it is valid and available in the registry. If not, the "open" verb is used. For Windows 2000 and later systems, the default verb is used if available. If not, the "open" verb is used. If neither verb is available, the system uses the first verb listed in the registry. We should probably use NULL in preference to "open".
Bill or Samir, could you check this into the branch as soon as possible?
Whiteboard: [adt1] ETA 4/18 → [adt1] [ETA 4/26]
Priority: -- → P1
fix was checked in on branch last week
Keywords: fixed1.0.0
Whiteboard: [adt1] [ETA 4/26] → [adt1]
verified fixed using 2002.04.29.08-1.0.0 (branch) commercial bits on win2k. tested using the URL orginally reported. adding verified1.0.0 keyword.
Keywords: pp, verified1.0.0
marking fixed
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Keywords: pp
It works on the 4/28 build. The 4/29 and 4/30 builds do not ever start for me.
Reopening. The 'fix' for this bug is unacceptable in the Mac. If you want to address a Win32 specific problem please fix it in a manner that doesn't screw the Mac.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
So why is naming a RealAudio media file with extension ".ram" rather than ".exe" so unnacceptable to "the Mac?" Did you read the discussion of this issue above? Do you have counter-arguments? As it happens, we debated this fix with more than one Mac user and they didn't feel particularly screwed by it. So if we're just tallying votes, you lose, 2 to 1. We need more constructive input.
Upwards of 99% of the files Mac users download will be of the type application/octet-stream. This 'fix' seesm to be the root cause of all these files now sporting a '.binary' extension on top of whatever extension they already had. The Mac file system APIs currently in use limit the file names to 31 characters and you just truncated the allowable name length to 24 characters to accomodate the '.binary' extension. How is that considered acceptable, or useful? Change the name however you like on other platforms but leave them alone on the Mac is what I'll give you as my constructive input.
make that two on dagley's side. i find this patch aggrivating on macos (and yeah, i did read the bug). will i now find myself ending up with files on my desktop like "foo.mpeg.mpg" because our comparisons are lax with adding the suffix? When we get a particular mimetype, that should dictate the type/creator that the file gets saved as on mac, not the extension. I don't want you messing with my extensions. I don't want you to add .txt to the end of all my text files. Mac users are fairly picky about such things. I see no need to "correct" the extension on macOS, and doing so only gives a bad user experience.
To add to the mix, what about Mac OS X? I believe associations of a file to an app are made based on the extension on Mac OS X. Emperical observation: changing a file named ``foo'' to ``foo.txt'' and then double-clicking it results in the file being opened with TextEdit. Should we deal with files on Mac OS X in the same fashion we deal with files downloaded on Unices/Windows giving importance to the ``most relevant'' MIME type <-> extension mapping?
It's not so simple as you might think. Mac OS X is in a state of flux as it supports both the classic Mac type/creator system and file anme extensions. How things will settle out isn't clear yet but for now the type/creator system has precedence over the extension system. Where you see the latter coming into play is when an app doesn't provide any type/creator info.
comment 77 is not relevant What I don't understand about this bug is we can't compare the "desired extension" in a more general fashion (get its mimetype and see if the current extension is also of the same mimetype). Ideally the code would be architected to maintain the mimetype rather than trying to maintain a desired extension. Or am I missing something?
Can a single extension map to multiple MIME types on Windows?
Could someone please open up a separate, Mac-specific bug for this? I'd rather not morph it to cover the side effect.
should we just continue this current mac issue in bug 141330?
re-resolving as FIXED; the change has been undone on the Mac per bug 141330.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
There is still a problem. I tried to download an rpm (redhat) file from windows http://www.dtek.chalmers.se/groups/dvd/srpm/libdvdcss-0.0.3.ogle3-1.src.rpm and it opened in sone sort of real player screen!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
That's a problem with the server, which sends the audio/x-pn-realaudio-plugin MIME type for that file. The only correct thing to do there is to pass that content off to RealPlayer (and NS4 does exactly the same thing). Re-resolving.
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
Sairuh... This bug was reopened a few times. Is it still fixed on the branch? Need bug Verified on Mac as well
yes, this looks good on the branch: vrfy'd fixed using 2002.05.06.0x-1.0.0 comm bits on linux rh7.2, win2k and mac 9.1. double-checked this on the trunk as well using somewhat older builds (which should still have the fix): win2k, 2002.04.30.10-trunk mac 10.1.4, 2002.05.02.03-trunk linux rh7.2, 2002.04.30.10-trunk
Status: RESOLVED → VERIFIED
Keywords: verified1.0.0
*** Bug 144629 has been marked as a duplicate of this bug. ***
I think this fix may have broken the mail attachments. I got sent a registry key file (.key) to unlock software as an e-mail attachment. But when I saved it, it stuck on an extra .exe extension, thus preventing the file from launching properly.
That's actually bug 120327.
I think this still needs to be reopened. I got an e-mail from Suse for security patches that had i386 Intel Platform: SuSE-8.0 ftp://ftp.suse.com/pub/suse/i386/update/8.0/n2/apache-1.3.23-120.i386.patch.rpm 58752b3a35523263428c325b340c9ae8 ftp://ftp.suse.com/pub/suse/i386/update/8.0/n2/apache-1.3.23-120.i386.rpm b52837fe3f8512155ae93f7462526841 When I clicked on the ftp link to download it,guess what happened? It launched the RealPlayer plugin in my mail window and tried to play the file. 1) The real plugin should not be launched in the mail window (There was another bug about the pdf plugin being launched there which got fixed). 2) It should check to be sure it is a real file and not a rpm patch!
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
That's not this bug. Item #1 is a general bug about plugins in mail (already filed). Item #2, there is no really good way of doing that (it's a matter of whether we trust our content-sniffing or the system's extension-to-type mappings more, and since out content-sniffing is fairly primitive and out of date by definition we go with the system mappings first). In either case, the changes would be completely unrelated to the original bug reported here. Reclosing this one since the original problem is fixed.
Status: REOPENED → RESOLVED
Closed: 23 years ago22 years ago
Resolution: --- → FIXED
re-vrfy'ing...
Status: RESOLVED → VERIFIED
As noted in another bug, this fix is the wrong one. Mozilla should never ShellExecute a file downloaded from the network...EVER. That is the bug here. Mozilla should only offer to pass the file's location on to an application registered to handle the type of data that the server said it was (content-type).
> Mozilla should only offer to pass the file's location on to an application > registered to handle the type of data that the server said it was (content- > type). There is no such concept ("application registered to handle a content-type") on Windows as far as I have been determined. If there is, could you let us know how it can be accessed via the Win32 API? That would be very useful. Does the registry entry for an extension contain the exact path of the thing that will launch it? Is this format constant across Win32 versions? Please file a _separate_ bug on this issue, Jerry. Cc me on it. I'd very much like to see this fixed, but discussing it in 2 different bugs that are primarily concerned with other things is a sure way to get it _not_ to happen.
But, Mozilla should only run things listed in the helper applications list, but offer launch a wizard to add this new mime type and player to the list.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: