Closed Bug 271732 Opened 16 years ago Closed 16 years ago
.COM is overwritten by downloading the pif file
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041124 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20041124 On Win9x, COMMAND.COM is overwritten by downloading the pif file. The pif file is the shortcut of MS-DOS prompt. (That is in Accessaries folder of Start menu of Win9x) Reproducible: Always Steps to Reproduce: Server Side 1. Copy to any folder the shortcut of MS-DOS prompt. 2. Upload the shortcut to web server. I'm uploaded the file, here. http://www.d-toybox.com/mozilla/testpages/bug4103/test.pif Client Side 1. Show Context Menu on the above link. 2. Select [Save link target as...] 3. Select file path. Actual Results: Mozilla & Firefox will overwrite 'COMMAND.COM'. Expected Results: Mozilla & Firefox should be save to selected path. It is not reproduced by left click the link and saving. It is reproduced only using [Save link target as...].
I tested on WinMe.
Anyone know why save-as would be different from a unknown-type handler save? Darin (and Johnny?) recently changed something here to fix a different bug where one trusted the MIME type and the other went with the extension. But this sounds more like we're cracking open the pif and treating it as a link.
> Anyone know why save-as would be different from a unknown-type handler save? They use different code-paths for sure. In the save-as case, we know that the file is going to be saved (see contentAreaUtils.js), so we don't need to route it through the URI loader, etc. Instead, we do what we need to do to figure out where to save it, and then we use nsIWebBrowserPersist to actually save it.
Odd. This works fine on Linux, and for save link as all we'd be doing is grabbing the raw data and saving it in the persistence object, I would think... Could it be that if the .pif already exists trying to open the relevant file traverses through the pif on windows?
it would, pifs are treated like lnks, they're shortcuts. i'd like to cc two people who've been playing w/ the lnk resolver lately if that's ok w/ people...
We certainly need to cc people familiar with file stuff on windows if they're not already cced.
Unfortunately I don't have access to a Win9x box to test this at the moment. The win32 nsILocalFile implementation explicitly resolves only *.lnk files as shortcuts. (1) Does the client step "3. Select file path" require selecting any particular file path for download? i.e. the path to download to must be the path with command.com in it, or the path with an existing test.pif file in it? (1) http://lxr.mozilla.org/mozilla/source/xpcom/io/nsLocalFileWin.cpp#289 http://lxr.mozilla.org/mozilla/source/xpcom/io/nsLocalFileWin.cpp#525
I tried this on Windows 2000 and Windows 95. I downloaded test.pif twice on each platform. At no time was command.com overwritten.
I've just tested on Win98 SE, Firefox 1.0. Downloaded test.pif using "Save link as..." and left clicking (using the link in this bug report). Downloaded to c:\, c:\windows and desktop, but I can't reproduce the problem. Masayuki, would you please provide more detailed instructions for how to reproduce this problem.
Realy? I tested on WinMe. And original reporter in bugzilla-jp tested on WinMe too. This problem is on WinMe Only?? Win 95 OK Win 98 not tested Win 98SE OK Win ME NG Win 2k OK I don't have more detail. Because on WinMe, I can reproduce with the steps of first comment.
OS: Windows 98 → Windows ME
Just tested on a fresh install of WinME (English). No patches from Windows Update had been installed. Download of test.pif using Firefox 1.0, both left click and "save link as" were tested. I can't duplicate this problem. I can't see how this problem could occur. You will need to provide more detailed steps to reproduce this. I wonder if it is due to some way you have your WinME machine configured.
We(bugizlla-jp) tested this bug. Win95 not tested Win98 OK Win98 SE NG WinMe NG Win2k OK Umm... more detail? My WinMe system is on VMWare and without patchs. That is installed c:\windows(default path) on virtual machine. Mozilla & Firefox1.0 are installed each default install path.
Both WinME and Win98SE for me are running under MS Virtual PC (similar to VMware). I can't duplicate the problem with these instructions. From comment #11: "You will need to provide more detailed steps to reproduce this." Otherwise WORKSFORME.
> Otherwise WORKSFORME. wait a moment. This problem is reproduced on the system of 3 people(includeing me). http://www.d-toybox.com/mozilla/testpages/bug4103/test.pif 1. Move to cursor above link. 2. Click right button of the mouse. 3. Select [Save Link Target As...]. 4. Select saving path(anywhere). 5. We can see the worning dialog of overwrite to command.com.
I just realized that my build was patched to fix a similar bug. The patch to widget/src/windows/nsFilePicker.cpp is this: - ofn.Flags = OFN_NOCHANGEDIR | OFN_SHAREAWARE | OFN_LONGNAMES | OFN_OVERWRITEPROMPT | OFN_HIDEREADONLY | OFN_PATHMUSTEXIST; + ofn.Flags = OFN_NOCHANGEDIR | OFN_SHAREAWARE | OFN_LONGNAMES | OFN_OVERWRITEPROMPT | OFN_HIDEREADONLY | OFN_PATHMUSTEXIST | OFN_NODEREFERENCELINKS; The point of this extra flag is to stop Windows from downloading the target of a .url file but it might well be applying here too.
I tried backing my change out but it doesn't seem to affect this bug.
Okay. I finally reproduced it. The steps to reproduce are the same as comment #14 with the exception of step 4. The directory that you are downloading test.pif to must already have a copy of test.pif in it. The newly downloaded file is dereferencing the file refered to by the existing pif file. Thus, if the existing pif file points to c:\windows\command.com, and that file exists, then it will ask if you want to overwrite that file. If the existing pif file points to "c:\foo\bar.txt" and that file doesn't exist, you'll get a "The above file name is invalid" error. Duplicated with Firefox 1.0 and Suite 1.8a5 on Windows ME. All versions are English. I don't have a dev environment for Moz at the moment. Neil, would you try reproducing with your unpatched source first, and then try the patch and see if it fixes it?
Status: UNCONFIRMED → NEW
Ever confirmed: true
I just tested the change that Neil suggested and found that it does fix this bug for me (latest nightly Mozilla + Windows 98 and Windows ME). Neil - when do you expect to check this change in for your other bug?
That was bug 251651, but I never got around to discussing the solution...
Component: File Handling → GFX: Win32
The problem with OFN_NODEREFERENCELINKS is that it causes the save dialog to not follow shortcuts (.lnk files) to other directories either. Is the same problem present with a .lnk file? I don't know if there's a way to make the dialog not dereference only links to files.
(In reply to comment #20) > I don't know if there's a way to make the dialog not dereference only links to > files. Neither do I. Would we be able to manually hook into the dialog and do the resolution ourselves, based on whether we're opening or saving?
(In reply to comment #20) >The problem with OFN_NODEREFERENCELINKS is that it causes the save dialog to >not follow shortcuts (.lnk files) to other directories either. Is the same >problem present with a .lnk file? I don't know if there's a way to make the >dialog not dereference only links to files. Without OFN_NODEFERERENCELINKS: If I go to open web location, choose file, click on Favorites\MSN then IE downloads MSN to a temporary file whose location is returned to the open web location dialog. If I go to Shortcut to C:\ then the display changes to C:\ With OFN_NODEFERERENCELINKS: If I choose Favorites\MSN then Favorites/MSN.url is returned to the open web location dialog. If I go to Shortcut to C:\ then the display changes to C:\ Or am I missing something?
Does it still work if you make it a shortcut to a directory under C:, say C:\Temp? Sorry for not checking myself, but I'm on a new machine and don't have a dev environment set up on it yet.
I was testing save dialog, which is the culprit here, right? Open dialog might or Mozilla after getting the result might act differently.
Ah, it's in fact links to files that cause the problem. If I open a link to index.htm I get an error. If I save to a link to index.htm it overwrites index.htm.lnk
My test results. See http://jellycan.com/etc/test.html for the source page I used. The .lnk file links to c:\temp. The .pif file links to c:\windows\command.com. The .url file links to http://jellycan.com/etc/test.html Results ======= lnk: can .lnk files be followed to a different directory exists: what happens when a file of the same name exists in the destination directory. If follow, then we follow the link from the file. If replace then we replace the link file. FF1: Firefox 1.0 IE6: 6.0.2800.1106 (latest version from Windows update) Note: If a file of the same name doesn't exist, then in all cases we download to that directory. All saves were done using "save link as". All OS are updated to date using Windows Update. .pif ==== FF1, W2K: lnk: possible. exists: replace. FF1, ME/98: lnk: possible. exists: follow. IE6, W2K/ME: lnk: possible. exists: replace. IE6, 98: lnk: possible. exists: follow. .lnk ==== FF1, W2K/ME/98: lnk: possible. exists: follow. IE6, W2K/ME: lnk: NOT possible. exists: replace. IE6, 98: lnk: possible. exists: follow. .url ==== FF1, W2K: lnk: possible. exists: follow ("test.url" not valid) FF1, ME: lnk: possible. exists: follow ("http://link" not valid) FF1, 98: lnk: possible. exists: replace. IE6, W2K/ME/98: lnk: possible. exists: replace. The differences between OS platforms is interesting. In the .pif and .lnk cases IE has differences between ME and 98 that I didn't expect. This may infer that there is a way to prevent the following of links on ME but not on 98. I can't see any details of this in the open/save dialog documentation though. Note: I don't see this bug as a major security risk. The user needs to download a pif file into the same directory as an existing pif file. Trying to exploit that is shooting arrows at the moon.
Would it be correct to summarize this problem as: If you download a shortcut (.pif, .lnk, .url) file from a website into the same location, the second download attempt may result in the target of the shortcut being destroyed?
Yes. Download and save to disk of a file with the same name as an existing lnk/pif/url file will on some OS overwrite the target of the link instead of the source file. Only the OS/browser combinations with "exists: follow" in my test cases exhibit the problem. .pif = ME/98 .lnk = All .url = W2K/ME
The bad guy basically has a site that changes the mime type to something that causes the unknown content handler to kick in and ask the user to save the content to the desktop. This happens twice, and the bad guy can put any file anywhere on the users machine. Sounds bad. I think losing the ablity to follow shortcuts in our dialogs is equally as bad. The simpliest thing to do is to ensure that you can not replace a shortcut via the filepicker. This follows as you can't replace a folder. Thoughts?
Drivers think we need this bug fixed for the branches. Plussing. The fix suggested in comment 29 sounds reasonable. Doug, any chance you can come up with a patch?
i think I have something that might work. I need a win me box for testing. I will see if I can get a box. if you have one, please let me know.
Assignee: file-handling → dougt
On XP, GetSaveFileName never returns the dereferenced selected item. However, in SE, GetSaveFileName does return the dereferenced selected item. This patch will pass OFN_NODEREFERENCELINKS whenever we ask the user to select a place to download a file into. So, if the user ever downloads a shortcut, it will end up exactly where the user intented it to be. The drawback is that if the user wanted to download anything and selected an shortcut via this dialog, the download will ask the user if they want to replace the shortcut (no dereferencing happens). In this case, the user would have to double click the shortcut to open it in the dialog. I think that this would be the desired behavior of a typical save-as dialog on windows. This patch only changes the saving dialog; this patch does not change the open dialog.
Comment on attachment 173667 [details] [diff] [review] patch v.1 sr=dveditz
does this mean .lnk files pointing to directories no longer work in the dialog?
the short answer is .lnk files will continue to work. without this patch and on 98ish(*) machines, if you select a shortcut (.lnk) file in this dialog, the downloaded file will be placed in the directory which the shortcut points to. As you can see, this might now work if the shortcut is pointing at a file. You can still double-click on shortcuts to folders in the dialog.
Doug: what are the results of the testcases from comment #26 with your patch?
only tested 98 se. I verified that I could reproduce the problem using FF1.0. Then I made this one change, and tested again. With this patch, i was unable to have command.com be overwritten. One thing I did observe was that without this patch, the file that was downloaded and displayed in the Download Manager was "Command.com. Whereas on XP, this file's name was "test.pif". After this patch was applied, the file name that was downloaded was "command.com".
I see that this fix would prevent the following of the link files, however we should ensure that the patch doesn't have side-effects other than this, e.g. as noted in comment #20. How about testing all 3 file types on different operating systems? Through those test case results we can see that there are different results depending on the OS for this bug. The proposed fix should be tested similarly to prove that it does what is desired before being labelled as OK.
I think that based on my observations, there is an inconsistency in the WIN32 api. This patch does fix that. Comment #20 is somewhat unclear. I think neil is talking about open (not saving) files in IE or something. i need help testing all of these different os configurations.
Attachment #173667 - Flags: review?(bryner) → review+
Comment on attachment 173667 [details] [diff] [review] patch v.1 a=dveditz for 1.7 and aviary1.0.1 branches
Comment on attachment 173667 [details] [diff] [review] patch v.1 a=dveditz for 1.8b too
Attachment #173667 - Flags: approval1.8b+
fixed on branches and trunk.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Hello Masayuki, Does this look fixed to you using a recent 1.8 nightly build on WinME? ftp://mozilla.isc.org/pub/mozilla.org/mozilla/nightly/2005-02-17-14-trunk/
Yes, looks good on latest builds.
thank you for checking! verifying per reporter.
Status: RESOLVED → VERIFIED
This fix *DID* break the ability to browse shortcuts to other directories. See bug 283730.
On reading Microsoft's documentation of the Windows Common Dialog, it is supposed to behave the way it does in bug 271732. The common dialog only returns the name of the file or directory that a user chooses. It is then up to the application to do whatever with that information. If the user chooses a link, then the target of the link is returned. I see nothing wrong here. I did some testing. When you try to overwrite a link file with a downloaded file, it *does* then overwrite the target of the link, ***BUT*** it prompts you. It says, "XXXXX ALREADY EXISTS. DO YOU WANT TO REPLACE IT?" The XXX it displays is the name and path of the actual file it is going to replace. Where's the problem?
I think some user does not know the extension. Because the default Windows setting is that the extension is hidden. So, if the user download "command.pif", the user cannot understand it is system file or the selected file.
I think that many users don't know the file is system file or not.
I think this caused Bug #287717
You need to log in before you can comment on or make changes to this bug.