Firefox on Windows 10 is not dropping high privileges

VERIFIED FIXED in Firefox 62

Status

()

defect
P1
normal
VERIFIED FIXED
a year ago
8 months ago

People

(Reporter: haenel, Assigned: aklotz)

Tracking

(Blocks 1 bug, {sec-want})

Trunk
Firefox 62
Unspecified
Windows
Points:
---

Firefox Tracking Flags

(firefox62 verified)

Details

(Whiteboard: [adv-main62-] inj+)

Attachments

(1 attachment)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.132 Safari/537.36

Steps to reproduce:

Steps to reproduce the problem:
1. Close all your firefox browsers (and make firefox your default app to open web pages, just to be sure).
2. From a UAC elevated application, open a chrome browser (it can be an UAC elevated application with a link, for example, an installer).
3. The firefox browser that is spawned is elevated.



Actual results:

The privileges should be dropped. This is for example the behavior of Microsoft Edge or Opera.



Expected results:

The firefox browser continues to run with high privileges.

It can be checked by checking the Security tokens (I used Process Explorer for that), but that is not always “clean cut”.
What is clearer, is spawning a cmd.exe from the browser: if the browser is privileged, the cmd.exe will be “Run as administrator” with the text “administrator” as part of the title bar. If the browser dropped the privileges, the prompt will be a regular one.
For example: Save file -> Browse to Windows\system32 -> file cmd.exe, right click -> Open (NOT Open as administrator).
[Of course, this is just the symptom, not the real risk].
(Reporter)

Comment 1

a year ago
Two important points: 
- Scenarios where this can happen include simple installation of third party apps, installed for all users. I tried with IrfanView (first installer I tried to look for this behavior): The installer requests UAC elevation, and then starts the installation. On the first part of the installation, there is a link to the EULA, and when clicked, it spawns the default browser. 
If the browser is Firefox and no other Firefox windows was opened at this stage, Firefox will continue to run HighIL privilege ( S-1-16-12288 ). 

- The renderers are running at LowIL (good), BUT, any file downloaded by any tab, clicked from the Download (Ctrl-J) page, are run with HighIL, without requesting UAC (since it just inherited the privileges)!
Jim, is this something somebody on your team could triage? Thanks.
Flags: needinfo?(jmathies)
This is because we use requestedExecutionLevel asInvoker in the manifest, which is the least privilege you can request using this method I believe.

At the moment this is expected behaviour, but it would be great to fix this.
I think we will need a launcher/bootstrap process.
I'm not totally sure this bug needs to be hidden.

Chrome runs as administrator for the same reason I think.

Opera uses some sort of launcher to start a medium integrity process.
Not quite sure how it does this, it starts a few processes as straight forward children at high, but the final medium integrity main process doesn't get caught by windbg's child process handling.

Edge is launched in a different way, because it's a system app.
Per Bob's comments, this is something we can look at in our inject eject work.

Dan, can you please kick this out of security bugs?
Blocks: injecteject
Group: firefox-core-security → core-security
Status: UNCONFIRMED → NEW
Component: Untriaged → DOM: Content Processes
Ever confirmed: true
Flags: needinfo?(jmathies) → needinfo?(dveditz)
OS: Unspecified → Windows
Product: Firefox → Core
Priority: -- → P3
Group: core-security
Flags: needinfo?(dveditz)
Keywords: sec-want
(Assignee)

Updated

a year ago
Blocks: 1435780
No longer blocks: injecteject
(Assignee)

Updated

a year ago
Whiteboard: inj+
(Assignee)

Comment 5

a year ago
This patch follows Raymond Chen's guidelines for re-launching a process via Explorer.

While I expect there to be a perf penalty, I think that it is acceptable in this case because we're trying to launch Firefox from an elevated user.

We delayload COM/OLE DLLs to avoid paying a perf penalty if the process is being launched as a normal user.
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Attachment #8973847 - Flags: review?(jmathies)
(Assignee)

Comment 6

a year ago
Moving components, as this is for firefox.exe launching the browser process.
Component: DOM: Content Processes → General
Product: Core → Firefox
Version: 57 Branch → Trunk
Doesn't it make an infinite loop when WinXP or RunAsAdmin compatibility shim is applied?
(Assignee)

Comment 8

a year ago
Potentially, I guess. Right now it doesn't matter too much because these code paths do not run by default. We can handle that in a follow-up.
Comment on attachment 8973847 [details] [diff] [review]
If the launcher process is being run at high integrity, find Explorer and command it to ShellExecute

Review of attachment 8973847 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm!

::: browser/app/LaunchUnelevated.cpp
@@ +37,5 @@
> +{
> +  // We require a single-threaded apartment to talk to Explorer.
> +  mscom::STARegion sta;
> +  if (!sta.IsValid()) {
> +    return false;

I think we'll need some debugging output for all this code eventually.. something more advanced than simple printfs to the command line in debug builds. Some sort of logging module maybe.. thoughts?
Attachment #8973847 - Flags: review?(jmathies) → review+
(Assignee)

Comment 10

a year ago
Yeah, in the main launcher process source, I added some stuff that does a FormatMessage + MessageBox for error codes. I'll probably hook this stuff in at the very least.

Maybe the Windows event log?

We can think about that in a follow up.
(Assignee)

Comment 11

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/0e851b076fddb5eee96c6349ab21479b23ecfc7a
Bug 1430092: Make the bootstrap process re-launch itself it it is running at a high integrity level; r=jimm
(Assignee)

Comment 12

a year ago
(In reply to Aaron Klotz [:aklotz] from comment #8)
> Potentially, I guess. Right now it doesn't matter too much because these
> code paths do not run by default. We can handle that in a follow-up.

I filed bug 1460434 for this.

(In reply to Aaron Klotz [:aklotz] from comment #10)
> Yeah, in the main launcher process source, I added some stuff that does a
> FormatMessage + MessageBox for error codes. I'll probably hook this stuff in
> at the very least.
> 
> Maybe the Windows event log?
> 
> We can think about that in a follow up.

And bug 1460433 for this.
(Assignee)

Updated

a year ago
Priority: P3 → P1

Comment 13

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/0e851b076fdd
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
Flags: qe-verify+
Managed to reproduce using Firefox 58.0a1 (2017-11-01), 61.0b7.

Have checked with Firefox 62.0b7 on win 10x64  using IrfanView (as described in comment 1).
The issue appears to still be reproducible, Firefox being launched with elevated status.
Flags: needinfo?(aklotz)
(Assignee)

Comment 15

9 months ago
This patch has landed as part of the launcher process, which is not yet enabled by default.

In order to test this, you must start firefox with the --launcher flag on the command line.
Flags: needinfo?(aklotz)
(In reply to Aaron Klotz [:aklotz] from comment #15)

> In order to test this, you must start firefox with the --launcher flag on
> the command line.

Wouldn't that just open up another instance; not linked to the elevated application?
Flags: needinfo?(aklotz)
(Assignee)

Comment 17

9 months ago
We don't want Firefox to be associated with the elevated application at all. In fact, the way this is implemented in the code is to request Explorer to start the new instance on our behalf, explicitly so that there aren't any links.
Flags: needinfo?(aklotz)
(Assignee)

Comment 18

9 months ago
Replying to Slack questions from cfogel here:

> - setting the launcher flag, I've read somewhere that it gets reset after closing Firefox.

Currently Firefox must be started with --launcher every time you want to test this functionality. Eventually this feature will probably become the default and the flag will no longer be needed, but we must to do more work to get to that point.

> - the new instance is launched from opening the link, not the console. Right?

I wouldn't worry too much about the wording of the STR in the description. What we really want to test is what happens when we execute "firefox --launcher" from within an application that is already elevated. The easiest way to do that is from an Administrator command prompt.
Thank you for the prompt replies, I was to hung up on the STR indeed.

Managed to reproduce the issue with 61.0b14.
Have checked with Firefox 62.0b9 on Win_10x64 and Win_8.1Pro; can confirm the issue has been fixed based on the previous comment.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: inj+ → [adv-main62-] inj+
You need to log in before you can comment on or make changes to this bug.