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

RESOLVED FIXED

Status

()

Toolkit
Build Config
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: emorley, Assigned: emk)

Tracking

unspecified
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

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

Details

Attachments

(5 attachments, 2 obsolete attachments)

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.
(Reporter)

Comment 1

7 years ago
Created attachment 446858 [details]
Screenshot of comptibility mode assistant prompt
(Reporter)

Updated

7 years ago
Attachment #446858 - Attachment description: Comptibility mode assistant dialogue → Screenshot of comptibility mode assistant prompt
(Assignee)

Comment 2

7 years ago
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
(Assignee)

Comment 3

7 years ago
This problem doesn't occur if I updated the manifest to Win7.
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 522065
(Assignee)

Comment 4

7 years ago
Sorry, this bug was about firefox.exe itself (not about the installer). Reopening.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(Assignee)

Comment 5

7 years ago
Created attachment 446884 [details] [diff] [review]
adding manifest to Win7
Assignee: nobody → VYV03354
Status: REOPENED → ASSIGNED
Attachment #446884 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

7 years ago
Assignee: VYV03354 → nobody
Component: Download Manager → Build Config
QA Contact: download.manager → build-config
(Assignee)

Comment 6

7 years ago
Created attachment 446885 [details] [diff] [review]
Also changed manifests for binaries other than firefox.exe
Attachment #446884 - Attachment is obsolete: true
Attachment #446884 - Flags: review?(ted.mielczarek)
(Assignee)

Updated

7 years ago
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.
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
(Assignee)

Comment 8

7 years ago
Created attachment 446905 [details] [diff] [review]
comm-central counterpart
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.

Updated

7 years ago
Assignee: nobody → VYV03354
(Assignee)

Comment 13

7 years ago
Created attachment 447097 [details] [diff] [review]
Add more files [Checkin: Comment 14]

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)
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Attachment #447097 - Flags: review?(ted.mielczarek) → review+
(Assignee)

Updated

7 years ago
Keywords: checkin-needed
Pushed as: http://hg.mozilla.org/mozilla-central/rev/63b89b311461
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
Blocks: 570365
(Assignee)

Updated

7 years ago
Attachment #447097 - Attachment description: Add more files → Add more files [Checkin: Comment 14]
(Assignee)

Comment 15

7 years ago
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]
(Assignee)

Comment 17

7 years ago
Created attachment 452427 [details] [diff] [review]
1.9.2 branch patch

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?
(Assignee)

Comment 18

7 years ago
Created attachment 452428 [details] [diff] [review]
1.9.1 branch patch

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!

Updated

7 years ago
Attachment #452427 - Flags: approval1.9.2.7?
Attachment #452427 - Flags: approval1.9.2.6-
Attachment #452427 - Flags: approval1.9.2.5?

Updated

7 years ago
Attachment #452428 - Flags: approval1.9.1.12?
Attachment #452428 - Flags: approval1.9.1.11?
Attachment #452428 - Flags: approval1.9.1.11-

Comment 20

7 years ago
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 24

7 years ago
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-

Updated

7 years ago
Attachment #452428 - Flags: approval1.9.1.12+ → approval1.9.1.12-
Attachment #452427 - Flags: approval1.9.2.10?
Attachment #452428 - Flags: approval1.9.1.13?
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.

Updated

7 years ago
Attachment #452427 - Flags: approval1.9.2.11? → approval1.9.2.11+

Updated

7 years ago
Attachment #452428 - Flags: approval1.9.1.14? → approval1.9.1.14+
(Assignee)

Updated

7 years ago
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
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/9fc9768e25c0
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5068a37ab3c8
status1.9.1: --- → .14-fixed
status1.9.2: --- → .11-fixed
Keywords: checkin-needed
Whiteboard: [1.9.1 and 1.9.2 branch]

Comment 29

7 years ago
Pushed to comm-1.9.1 a=Callek.
http://hg.mozilla.org/releases/comm-1.9.1/rev/f1de05cd3b11
Depends on: 669463
You need to log in before you can comment on or make changes to this bug.