1643199 - (CVE-2020-15663) Mozilla Maintenance Service Privilege Escalation via updater.exe if Firefox is installed in non-default location

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect, P2)

Description

[Xiaoyin Liu]

Attachment: poc.zip

This report is inspired by CVE-2019-11753 (Bug 1574980) and CVE-2017-7766 (Bug 1342742).

Summary: If a user has installed Firefox to a standard-user-writable location, then a local attacker who already has non-admin privilege can escalate his privilege to SYSTEM.

Detail: Since the Firefox installation path is user-writable, a local attacker can replace any files in the installation path. What we need to replace are updater.exe and updater.ini. Although the Maintenance Service checks if updater.exe contains an identity string and is signed by Mozilla, it doesn't check its file version. Thus, we can replace the currently installed updater.exe with an old and vulnerable version of updater.exe, and, from what I observed, the Maintenance Service copies the old updater.exe and updater.ini to C:\Program Files (x86)\Mozilla Maintenance Service\update, and then runs the old updater.exe with SYSTEM privilege.

The old updater.exe I choose to use is version 51.0.1, so that I can exploit Bug 1342742 again. (Actually exploiting the first part of Bug 1342742 is all we need, because we can edit updater.ini directly.)

After updater.exe finishes applying update.mar, it runs the program specified in [PostUpdateWin] section in updater.ini. We specify a full path for ExeRelPath. It has to be Mozilla-signed, but since the attacker has total control of the directory, we can use DLL injection to run attacker's code. For example, we can place a malicious version.dll in the same directory as ExeRelPath. The code in version.dll will run with SYSTEM privilege.

Reproduce: (Test environment: OS: Windows 10 x64 Build 19041.264. Firefox version: 76.0.1)

1. Uninstall your current Firefox, and then install it to C:\Common\Mozilla Firefox
2. Log in a standard Windows user account
3. Create folder C:\test\poc
4. Unzip the attachment to C:\test\poc
5. Download https://archive.mozilla.org/pub/firefox/releases/51.0.1/update/win32/en-US/firefox-51.0.1.complete.mar, and save it to C:\test\poc\123\updates\0\update.mar
6. There are two files under C:\test\poc\copy to Firefox install dir: updater.exe and updater.ini. Copy them to your Firefox installation directory, overwriting the existing ones.
7. Open cmd.exe . Run sc start MozillaMaintenance MozillaMaintenance software-update unused C:\test\poc\123\updates\0 "C:\Common\Mozilla Firefox" "C:\Common\Mozilla Firefox\updated" -1 . C:\test\poc\uninstall.update
8. After a few seconds, you can see the poc creates a file "C:\DLLPlanting\dll_output.txt", whose content is: Filename=C:\test\poc\fakesetup.exe Integrity level=System, which proves the PoC is run with SYSTEM privilege.

Comment 1

[Xiaoyin Liu]

A simple fix would be to compare the version of updater.exe and maintenanceservice.exe before running it. updater.exe must be no older than maintenanceservice.exe.

Comment 2

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I believe this is the second scenario I described in bug 1588884 comment 3, a version check won't help with the first scenario there.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:nalexander, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Nick Alexander :nalexander [he/him] (PTO Feb 24-March 3)]

I think this feels like an S3: it's not critical but we should continue to lock down the Maintenance Service.

mhowell, this was assigned to you. Did you decide on an approach? Is it reasonable to follow rstrong's suggested first mitigation, namely to restrict the MS and/or updater to only updating files in ProgramFiles-like locations?

Comment 5

[Molly Howell (she/her) (mostly inactive)]

I've been working on implementing that mitigation, and it does seem to work, but making the tests happy about that change has taken some effort, that's the reason for the delay. It is possible though, just working through some last details.

Comment 6

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1643199 - Limit the install locations on which the maintenance service may be invoked. r=agashlin,bytesized

Comment 7

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9162642 [details]
Bug 1643199 - Limit the install locations on which the maintenance service may be invoked. r=agashlin,bytesized

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Certainly not directly, it's a pretty general mitigation against a whole class of Maintenance Service file privilege escalations (several of which have been patched before).
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: There is a small conflict with bug 1588549 that prevents the merges from being trivial, but it isn't difficult to resolve.
• How likely is this patch to cause regressions; how much testing does it need?: It's certain to cause some regressions because we're intentionally removing support for running the Maintenance Service outside of Program Files, so installations in that kind of configuration will start seeing UAC prompts when they didn't before, but should still be able to update.
  As for more widespread regressions, we have good test coverage, and I've been running the patched service myself while developing the patch without seeing any issues, so I'm confident. We should still give this patch two or three days on Nightly to see if issues do crop up before executing any uplifts.

Comment 8

[Daniel Veditz [:dveditz]]

Comment on attachment 9162642 [details]
Bug 1643199 - Limit the install locations on which the maintenance service may be invoked. r=agashlin,bytesized

sec-approval+

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/6a541f963216b68b88f16164bf11f93fdc7c9bd7
https://hg.mozilla.org/mozilla-central/rev/6a541f963216

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for uplift to Beta/ESR78 when you get a chance. It grafts cleanly to both. It's going to need a rebased patch for ESR68 and approval request when ready as well.

Comment 11

[Molly Howell (she/her) (mostly inactive)]

As I said in the sec-approval form, I want this to sit on Nightly for a few days to make sure that it at least doesn't cause anyone there any problems before it starts getting uplifts. So unless you disagree with that, I'd rather wait a couple of days before filing any uplift requests.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

The final scheduled Fx79 beta build is scheduled for tomorrow night PDT before RC week next week, so I guess that depends on how worried you are about that timeline.

Comment 13

[Molly Howell (she/her) (mostly inactive)]

I'll make the requests then (and then post the ESR68 patch), but I think this might be my first time using the "high" risk level in that form.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 15

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9162642 [details]
Bug 1643199 - Limit the install locations on which the maintenance service may be invoked. r=agashlin,bytesized

Beta/Release Uplift Approval Request

• User impact if declined: sec-high local privilege escalation bug
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): We do have automated test coverage around the maintenance service to the extent possible, and I've verified that my local build of the patch did not interfere with Nightly updates on my machine, but "works on my machine" isn't a very high bar.
  Fortunately since this patch is really only to the maintenance service, the risk is that the service refuses to run, which isn't fatal to installing updates; the user would see a UAC prompt, which would be confusing and a bit scary, but the update should succeed if they accept the prompt. That's the reason the risk isn't "high".
• String changes made/needed:

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high bug
• User impact if declined:
• Fix Landed on Version: 80
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): See above
• String or UUID changes made by this patch:

Comment 16

[Molly Howell (she/her) (mostly inactive)]

(In reply to Release mgmt bot [:sylvestre / :calixte / :marco for bugbug] from comment #14)

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

I've filled out the linked form. It doesn't say whether the whiteboard tag should be left or cleared, so to be safe I'm leaving it.

Comment 17

[Molly Howell (she/her) (mostly inactive)]

Attachment: ESR68 Patch

Same as the existing approved patch, rebased to esr68 and with the one merge conflict in updater.cpp resolved.

Comment 18

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9163828 [details] [diff] [review]
ESR68 Patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: See the beta and ESR78 requests in comment 15; this patch is identical except for a rebase and a small manual merge, so all responses there apply the same to this patch.
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 19

[Molly Howell (she/her) (mostly inactive)]

This patch is broken, my Windows installations are all getting UAC prompts today. I'm investigating but in the meantime I'm cancelling the uplift requests.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Given where we are in this cycle, I think we should just aim to have this fix ride with Fx80/78.2esr/68.12esr at this point. This is sounding really risky to take so close to RC week.

Comment 21

[Molly Howell (she/her) (mostly inactive)]

Attachment: Bug 1643199 Followup - Properly account for registry data length. r=agashlin,bytesized

Comment 22

[Molly Howell (she/her) (mostly inactive)]

I'm almost done editing the followup patch. It doesn't contain any changes to the security-relevant content, so in the interest of getting this fixed quickly, I intend to push it once it has r+ without waiting for sec approval.

Comment 23

[Molly Howell (she/her) (mostly inactive)]

https://hg.mozilla.org/integration/autoland/rev/be2431f1b1b925badbde8bcd5c6a38b902a2bae9

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/be2431f1b1b9

Comment 25

[Adam Gashlin (he/him) [:agashlin] (ex-moco)]

I ran across an issue at the intersection of twice-daily Nightly and our limited staging fallback logic, which caused the manual install prompt to show up today when updating a system that had previously been updated from 2020-07-17-09-39-07 to 2020-07-17-21-28-41.

Before the fix on Friday we would show the UAC prompt on startup, because the updater would refuse to use the maintenance service for staging due to the failed IsProgramFilesPath() check; that check is duplicated, along with several others, in both the updater and the service. If the updater expects the service will reject the command line, it sets the status to UNEXPECTED_STAGING_ERROR and Firefox falls back to trying a non-staged, non-service update on the next startup.

Because of bug 1654072, the maintenance service wasn't updated from 2020-07-17-09-39-07 to 2020-07-17-21-28-41, though the updater was. This meant that the updater thought that the IsProgramFilesPath() check would pass, so it tried to run the maintenance service, which still had the bug so it failed. The fallback is not done in this case, so the whole update process failed, causing the manual download prompt. I proposed to fix this part in bug 1654079.

I don't think this should require any more action here, it isn't going to be an issue at a slower build cadence, but it would be good to fix the linked bugs for more robust fallback and to prevent issues with updating the service in Nightly.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Is this ready for ESR uplift requests?

Comment 27

[Molly Howell (she/her) (mostly inactive)]

I believe so, I've seen no more reports of issues. I'll enter the request.

Comment 28

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9162642 [details]
Bug 1643199 - Limit the install locations on which the maintenance service may be invoked. r=agashlin,bytesized

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a sec-high bug.
• User impact if declined: Exposure to local privilege escalation under certain (likely uncommon) install configurations.
• Fix Landed on Version: 80
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This change originally caused one major issue, which the followup patch addresses. Since then no more reports of other issues have been received, from nightly or beta.
• String or UUID changes made by this patch:

Comment 29

[Molly Howell (she/her) (mostly inactive)]

Comment on attachment 9163828 [details] [diff] [review]
ESR68 Patch

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: See the ESR 78 request in comment 28.
• User impact if declined:
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch:

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9162642 [details]
Bug 1643199 - Limit the install locations on which the maintenance service may be invoked. r=agashlin,bytesized

Approved for 78.2esr and 68.12esr.

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/0e88d553e30c
https://hg.mozilla.org/releases/mozilla-esr78/rev/c757b6525567

https://hg.mozilla.org/releases/mozilla-esr68/rev/2e1e3cb4769c
https://hg.mozilla.org/releases/mozilla-esr68/rev/26fd82275fa5

Comment 32

[Rares Doghi, Desktop QA]

I tried to reproduce this issue on our end but I was unable to reproduce it using the reporters steps, Step 8 from the description does not occur.

Maybe someone can help us confirm that this issue is Verified as Fixed in our latest builds Nightly 81 as well as ESR 68 and 78 builds, and we can update the flags accordingly.

Comment 34

[Molly Howell (she/her) (mostly inactive)]

I did get the POC to work and create its file using an unpatched Nightly, build 20200601093812 [1]. After that, I rebooted, installed the current Nightly (20200812034418) instead, and reran the steps; this time the file was not created and the maintenance service log ended with the error message that the patch is expected to produce, which is "The install directory path is not valid for this application." Based on that, I believe the fix can be considered verified.

[1] It did take me a couple of attempts because I didn't initially notice that, since I used Nightly, the installation directory name was not the same as the one in the command in step 7. After correcting those paths, the POC ran as expected.

Comment 35

[Rares Doghi, Desktop QA]

Thank you for taking the time to Verify this fix in our latest Nightly build, can you please also verify this fix in our latest Beta and ESR builds and we will update the tracking flags accordingly.

Comment 36

[Molly Howell (she/her) (mostly inactive)]

I've now verified the fix on beta using 80.0b7 against 79.0b9. ESR is harder because releases with the patch aren't made yet, so I had to test the unpatched 68.11.0 and 78.1.0 against CI builds for 68 and 78, but I did also verify the fix using those. Results were the same as in comment 34 in all cases.

Comment 37

[Rares Doghi, Desktop QA]

Thank you for all the help, I will update the flags accordingly.

Comment 38

[Tom Ritter [:tjr]]

Attachment: advisory.txt