Closed Bug 567497 Opened 14 years ago Closed 14 years ago

Files executed via download manager cause Win7 compatibility mode to permanently apply to firefox.exe and not downloaded file

Categories

(Toolkit Graveyard :: Build Config, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(status1.9.2 .11-fixed, status1.9.1 .14-fixed)

RESOLVED FIXED
Tracking Status
status1.9.2 --- .11-fixed
status1.9.1 --- .14-fixed

People

(Reporter: emorley, Assigned: emk)

References

Details

Attachments

(5 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2.3) Gecko/20100401 Firefox/3.6.3

If downloaded executable files are launched via the Firefox download manager and the downloaded file triggers the windows compatibility mode assistant - then the compatibility assistant wizard sees Firefox as the owner process and so any changes it makes (eg "always run as administrator") are applied to firefox.exe and not just the downloaded file. These changes are then applied on every subsequent Firefox launch, and cause a UAC elevation prompt each time and the worrying side effect of permanently running Firefox as an administrator.

Reproducible: Always

Steps to Reproduce:
1. Launch Firefox and download an installer that triggers Windows Compatibility mode (eg this utility installer: http://support.microsoft.com/kb/290301)
2. Double click the file in the download manager to launch it
3. Click "Run" on the "are you sure you wish to run this file from internet zone" type prompt
4. Dismiss the subsequent access denied scripting host error (the installer will now exit, having failed)
5. Close Firefox
6. Once the Firefox process has closed, Windows will present the "Program Compatibility Assistant" dialogue (see attached jpg). The process is identified as firefox.exe and not the downloaded file.
7. Choose "Restart the program as an administrator"
8. The compatibility mode assistant will precede to restart Firefox and not the downloaded file as an administrator. This will permanently change the Firefox settings to run as admin, unless the user manually reverses this by un-ticking the "run as admin" box in the firefox.exe file properties.
Actual Results:  
Firefox.exe received the "run as administrator" compatibility mode corrections, rather than the downloaded file. This causes a UAC prompt on every subsequent Firefox startup, which aside from being annoying, means that Firefox is running unnecessarily as an administrator, which has fairly severe security implications.

Expected Results:  
Only the downloaded file has the "run as admin" correction applied and not firefox.exe. ie: Files launched via the download manager are launched in such a way that firefox.exe is not seen as the owner process, so the compatibility mode assistant can apply the corrections to the correct file. 

Could reproduce on FF 3.6.3 and Minefield nightly 3.7a5pre BuildID=20100521035955.

NB: After manually reversing the "run as admin" file compatibility properties, the wizard appears to not run again even if the STR are repeated. I presume there must be compatibility mode assistant cache in the registry that prevents the same action being applied again. However I found that by running a different installed copy of Firefox (eg Minefield nightly) rather than my main 3.6 install, the compatibility mode wizard would be triggered again. To prevent the compatibility mode wizard caching the result, just chose cancel at step 7 above, and the rest of the STR can be recreated as many times as required.
Attachment #446858 - Attachment description: Comptibility mode assistant dialogue → Screenshot of comptibility mode assistant prompt
Confirming.
If I launch Firefox from command line, I see the PCA dialog against the downloaded file before step 5. You need to launch Firefox from Explorer to reproduce the bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
This problem doesn't occur if I updated the manifest to Win7.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Sorry, this bug was about firefox.exe itself (not about the installer). Reopening.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Attached patch adding manifest to Win7 (obsolete) — Splinter Review
Assignee: nobody → VYV03354
Status: REOPENED → ASSIGNED
Attachment #446884 - Flags: review?(ted.mielczarek)
Assignee: VYV03354 → nobody
Component: Download Manager → Build Config
QA Contact: download.manager → build-config
Attachment #446884 - Attachment is obsolete: true
Attachment #446884 - Flags: review?(ted.mielczarek)
Attachment #446885 - Flags: review?(ted.mielczarek)
Attachment #446885 - Flags: review?(ted.mielczarek) → review+
I'm sure Thunderbird and SeaMonkey would like to similarly patch their manifests.
Keywords: checkin-needed
Attachment #446905 - Flags: review?(bugzilla)
(In reply to comment #7)
> I'm sure Thunderbird and SeaMonkey would like to similarly patch their
> manifests.

Thank you for the heads up!
Comment on attachment 446885 [details] [diff] [review]
Also changed manifests for binaries other than firefox.exe

Ted,
Should xulrunner/stub/xulrunner-stub.exe.manifest also have this change? (in case an XULRunner app wants to expose the Download Manager)
Attachment #446885 - Flags: feedback?(ted.mielczarek)
Comment on attachment 446905 [details] [diff] [review]
comm-central counterpart

Emk, first off thank you for the patch!

[Stealing review]

>+++ b/calendar/sunbird/app/sunbird.exe.manifest

Not necessarily needed anymore [Sunbird is no longer maintained standalone], but since its already patched happy to take it.

>+++ b/suite/app/seamonkey.exe.manifest

Heh, looks like SM missed some .manifest work already; Thanks for catching us up!
Attachment #446905 - Flags: review?(bugzilla) → review+
Note: If this needs backporting to comm-1.9.2, please make sure to the patch on a MailNews Core specific bug so that we can track its fix properly.
Assignee: nobody → VYV03354
Thanks for pointing out a missing file.
I've searched .exe.manifest from mxr.
http://mxr.mozilla.org/mozilla-central/find?text=&string=.exe.manifest
http://mxr.mozilla.org/comm-central/find?text=&string=.exe.manifest
I hope there are no missing files anymore...
Attachment #446885 - Attachment is obsolete: true
Attachment #447097 - Flags: review?(ted.mielczarek)
Attachment #446885 - Flags: feedback?(ted.mielczarek)
Keywords: checkin-needed
Attachment #447097 - Flags: review?(ted.mielczarek) → review+
Keywords: checkin-needed
Pushed as: http://hg.mozilla.org/mozilla-central/rev/63b89b311461
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Blocks: 570365
Attachment #447097 - Attachment description: Add more files → Add more files [Checkin: Comment 14]
Justin, did you check-in comm-central patch?
(In reply to comment #15)
> Justin, did you check-in comm-central patch?

Err no, whops; sorry.
Keywords: checkin-needed
Whiteboard: [c-n comm-central]
This fix should be landed on the branch along with bug 522065.
I think review is not needed because an only change is the path of updater.exe.manifest (toolkit/mozapps/update/updater/ -> toolkit/mozapps/update/src/updater/)
Attachment #452427 - Flags: approval1.9.2.5?
Removed missing files (plugin-container.exe.manifest and nsinstall.exe.manifest) only.
Attachment #452428 - Flags: approval1.9.1.11?
Have just tested this on the latest Minefield and can confirm this is now resolved for me.

Thanks!
Attachment #452427 - Flags: approval1.9.2.7?
Attachment #452427 - Flags: approval1.9.2.6-
Attachment #452427 - Flags: approval1.9.2.5?
Attachment #452428 - Flags: approval1.9.1.12?
Attachment #452428 - Flags: approval1.9.1.11?
Attachment #452428 - Flags: approval1.9.1.11-
We will not be taking this for .6/.11, moving nom for .7/.12
(In reply to comment #8)
> Created attachment 446905 [details] [diff] [review]
> comm-central counterpart

...I'm sorry... 

My neglect in pushing this live has caused this patch to bitrot, (I did not verify if someone did push and did not comment here though).

Within the next few days I'll *try* to find time to update the patch for you, but if you can manage doing that and re-add checkin-needed I can get this a bit faster for you.
Keywords: checkin-needed
Whiteboard: [c-n comm-central]
Comment on attachment 452427 [details] [diff] [review]
1.9.2 branch patch

Approved for 1.9.2.9, a=dveditz for release-drivers
Attachment #452427 - Flags: approval1.9.2.9? → approval1.9.2.9+
Comment on attachment 452428 [details] [diff] [review]
1.9.1 branch patch

Approved for 1.9.1.12, a=dveditz for release-drivers
Attachment #452428 - Flags: approval1.9.1.12? → approval1.9.1.12+
Comment on attachment 452427 [details] [diff] [review]
1.9.2 branch patch

Removing .9 &.12 approval as this didn't land before freeze. Feel free to nominate again, though the bar for approval will be higher.
Attachment #452427 - Flags: approval1.9.2.9+ → approval1.9.2.9-
Attachment #452428 - Flags: approval1.9.1.12+ → approval1.9.1.12-
Regrettable that these patches slipped through the cracks, but we still should take this.  It's relatively easy to hit, and painful for users that don't realize what's going on.
Attachment #452427 - Flags: approval1.9.2.11? → approval1.9.2.11+
Attachment #452428 - Flags: approval1.9.1.14? → approval1.9.1.14+
Keywords: checkin-needed
Whiteboard: [1.9.1 and 1.9.2 branch]
Callek <- TODO fix and checkin patch[es] to c-c.
(In reply to comment #26)
> Callek <- TODO fix and checkin patch[es] to c-c.

Err actually I just am overly forgetful lately:

http://hg.mozilla.org/comm-central/rev/8a82a37cab87

and *I* pushed it beginning of July http://hg.mozilla.org/comm-central/pushloghtml?changeset=8a82a37cab87
Keywords: checkin-needed
Whiteboard: [1.9.1 and 1.9.2 branch]
Depends on: 669463
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.