Last Comment Bug 567497 - Files executed via download manager cause Win7 compatibility mode to permanently apply to firefox.exe and not downloaded file
: Files executed via download manager cause Win7 compatibility mode to permanen...
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: x86_64 Windows 7
: -- normal (vote)
: ---
Assigned To: Masatoshi Kimura [:emk]
:
Mentors:
Depends on: 669463
Blocks: 570365
  Show dependency treegraph
 
Reported: 2010-05-21 19:50 PDT by Ed Morley [:emorley]
Modified: 2011-07-05 14:15 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.11-fixed
.14-fixed


Attachments
Screenshot of comptibility mode assistant prompt (50.82 KB, image/jpeg)
2010-05-21 19:56 PDT, Ed Morley [:emorley]
no flags Details
adding manifest to Win7 (964 bytes, patch)
2010-05-22 06:30 PDT, Masatoshi Kimura [:emk]
no flags Details | Diff | Splinter Review
Also changed manifests for binaries other than firefox.exe (3.89 KB, patch)
2010-05-22 06:52 PDT, Masatoshi Kimura [:emk]
ted: review+
Details | Diff | Splinter Review
comm-central counterpart (3.94 KB, patch)
2010-05-22 12:01 PDT, Masatoshi Kimura [:emk]
bugspam.Callek: review+
Details | Diff | Splinter Review
Add more files [Checkin: Comment 14] (7.78 KB, patch)
2010-05-24 09:55 PDT, Masatoshi Kimura [:emk]
ted: review+
Details | Diff | Splinter Review
1.9.2 branch patch (7.79 KB, patch)
2010-06-18 21:19 PDT, Masatoshi Kimura [:emk]
christian: approval1.9.2.11+
christian: approval1.9.2.7-
christian: approval1.9.2.9-
Details | Diff | Splinter Review
1.9.1 branch patch (5.02 KB, patch)
2010-06-18 21:21 PDT, Masatoshi Kimura [:emk]
christian: approval1.9.1.11-
christian: approval1.9.1.12-
christian: approval1.9.1.14+
Details | Diff | Splinter Review

Description Ed Morley [:emorley] 2010-05-21 19:50:14 PDT
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.
Comment 1 Ed Morley [:emorley] 2010-05-21 19:56:19 PDT
Created attachment 446858 [details]
Screenshot of comptibility mode assistant prompt
Comment 2 Masatoshi Kimura [:emk] 2010-05-22 04:10:42 PDT
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.
Comment 3 Masatoshi Kimura [:emk] 2010-05-22 04:26:02 PDT
This problem doesn't occur if I updated the manifest to Win7.

*** This bug has been marked as a duplicate of bug 522065 ***
Comment 4 Masatoshi Kimura [:emk] 2010-05-22 04:32:03 PDT
Sorry, this bug was about firefox.exe itself (not about the installer). Reopening.
Comment 5 Masatoshi Kimura [:emk] 2010-05-22 06:30:43 PDT
Created attachment 446884 [details] [diff] [review]
adding manifest to Win7
Comment 6 Masatoshi Kimura [:emk] 2010-05-22 06:52:27 PDT
Created attachment 446885 [details] [diff] [review]
Also changed manifests for binaries other than firefox.exe
Comment 7 Ted Mielczarek [:ted.mielczarek] 2010-05-22 09:43:32 PDT
I'm sure Thunderbird and SeaMonkey would like to similarly patch their manifests.
Comment 8 Masatoshi Kimura [:emk] 2010-05-22 12:01:00 PDT
Created attachment 446905 [details] [diff] [review]
comm-central counterpart
Comment 9 Justin Wood (:Callek) 2010-05-22 18:09:48 PDT
(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 10 Justin Wood (:Callek) 2010-05-22 18:12:22 PDT
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)
Comment 11 Justin Wood (:Callek) 2010-05-22 18:14:00 PDT
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!
Comment 12 Mark Banner (:standard8) (afk until 26th July) 2010-05-23 04:57:42 PDT
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.
Comment 13 Masatoshi Kimura [:emk] 2010-05-24 09:55:16 PDT
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...
Comment 14 Justin Wood (:Callek) 2010-06-05 10:21:03 PDT
Pushed as: http://hg.mozilla.org/mozilla-central/rev/63b89b311461
Comment 15 Masatoshi Kimura [:emk] 2010-06-11 23:17:51 PDT
Justin, did you check-in comm-central patch?
Comment 16 Justin Wood (:Callek) 2010-06-12 00:27:09 PDT
(In reply to comment #15)
> Justin, did you check-in comm-central patch?

Err no, whops; sorry.
Comment 17 Masatoshi Kimura [:emk] 2010-06-18 21:19:17 PDT
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/)
Comment 18 Masatoshi Kimura [:emk] 2010-06-18 21:21:07 PDT
Created attachment 452428 [details] [diff] [review]
1.9.1 branch patch

Removed missing files (plugin-container.exe.manifest and nsinstall.exe.manifest) only.
Comment 19 Ed Morley [:emorley] 2010-06-20 17:08:12 PDT
Have just tested this on the latest Minefield and can confirm this is now resolved for me.

Thanks!
Comment 20 christian 2010-06-24 13:48:36 PDT
We will not be taking this for .6/.11, moving nom for .7/.12
Comment 21 Justin Wood (:Callek) 2010-07-27 19:53:09 PDT
(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.
Comment 22 Daniel Veditz [:dveditz] 2010-08-06 11:05:59 PDT
Comment on attachment 452427 [details] [diff] [review]
1.9.2 branch patch

Approved for 1.9.2.9, a=dveditz for release-drivers
Comment 23 Daniel Veditz [:dveditz] 2010-08-06 11:06:20 PDT
Comment on attachment 452428 [details] [diff] [review]
1.9.1 branch patch

Approved for 1.9.1.12, a=dveditz for release-drivers
Comment 24 christian 2010-09-01 09:59:42 PDT
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.
Comment 25 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-01 10:15:03 PDT
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.
Comment 26 Justin Wood (:Callek) 2010-09-24 12:25:53 PDT
Callek <- TODO fix and checkin patch[es] to c-c.
Comment 27 Justin Wood (:Callek) 2010-09-26 22:11:28 PDT
(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
Comment 29 Philip Chee 2010-09-28 03:10:09 PDT
Pushed to comm-1.9.1 a=Callek.
http://hg.mozilla.org/releases/comm-1.9.1/rev/f1de05cd3b11

Note You need to log in before you can comment on or make changes to this bug.