Closed Bug 1643199 (CVE-2020-15663) Opened 2 years ago Closed 1 year ago

Mozilla Maintenance Service Privilege Escalation via updater.exe if Firefox is installed in non-default location

Categories

(Toolkit :: Application Update, defect, P2)

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
mozilla80
Tracking Status
firefox-esr68 80+ verified
firefox-esr78 80+ verified
firefox78 --- wontfix
firefox79 + wontfix
firefox80 + verified

People

(Reporter: xiaoyin.l, Assigned: mhowell)

References

(Regressed 1 open bug, )

Details

(Keywords: csectype-priv-escalation, sec-high, Whiteboard: [sec-survey][post-critsmash-triage][adv-main80+][adv-esr68.12+][adv-esr78.2+])

Attachments

(5 files)

Attached file 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.

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.

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.

Assignee: nobody → mhowell
Status: NEW → ASSIGNED
Priority: -- → P2

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(nalexander)

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?

Flags: needinfo?(nalexander) → needinfo?(mhowell)

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.

Flags: needinfo?(mhowell)

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.
Attachment #9162642 - Flags: sec-approval?

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

sec-approval+

Attachment #9162642 - Flags: sec-approval? → sec-approval+
Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

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.

Flags: needinfo?(mhowell)

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.

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.

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.

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.

Flags: needinfo?(mhowell)
Whiteboard: [sec-survey]

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:
Flags: needinfo?(mhowell)
Attachment #9162642 - Flags: approval-mozilla-esr78?
Attachment #9162642 - Flags: approval-mozilla-beta?

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

Flags: needinfo?(mhowell)
Attached patch ESR68 PatchSplinter Review

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

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:
Attachment #9163828 - Flags: approval-mozilla-esr68?

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.

Attachment #9162642 - Flags: approval-mozilla-esr78?
Attachment #9162642 - Flags: approval-mozilla-beta?
Attachment #9163828 - Flags: approval-mozilla-esr68?

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.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla80 → ---

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.

Regressions: 1653685
Status: REOPENED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla80

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.

Flags: sec-bounty? → sec-bounty+
Flags: qe-verify+
Whiteboard: [sec-survey] → [sec-survey][post-critsmash-triage]

Is this ready for ESR uplift requests?

Flags: needinfo?(mhowell)

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

Flags: needinfo?(mhowell)

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:
Attachment #9162642 - Flags: approval-mozilla-esr78?
Attachment #9164131 - Flags: approval-mozilla-esr78?

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:
Attachment #9163828 - Flags: approval-mozilla-esr68?
Attachment #9164131 - Flags: approval-mozilla-esr68?

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.

Attachment #9162642 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Attachment #9163828 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+
Attachment #9164131 - Flags: approval-mozilla-esr78?
Attachment #9164131 - Flags: approval-mozilla-esr78+
Attachment #9164131 - Flags: approval-mozilla-esr68?
Attachment #9164131 - Flags: approval-mozilla-esr68+
QA Whiteboard: [qa-triaged]

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.

Flags: needinfo?(mhowell)
Flags: needinfo?(agashlin)
Duplicate of this bug: 1588884

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.

Flags: needinfo?(mhowell)
Flags: needinfo?(agashlin)

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.

Flags: needinfo?(mhowell)

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.

Flags: needinfo?(mhowell) → needinfo?(rares.doghi)

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

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Flags: needinfo?(rares.doghi)
Whiteboard: [sec-survey][post-critsmash-triage] → [sec-survey][post-critsmash-triage][adv-main80+]
Whiteboard: [sec-survey][post-critsmash-triage][adv-main80+] → [sec-survey][post-critsmash-triage][adv-main80+][adv-esr68.12+]
Whiteboard: [sec-survey][post-critsmash-triage][adv-main80+][adv-esr68.12+] → [sec-survey][post-critsmash-triage][adv-main80+][adv-esr68.12+][adv-esr78.2+]
Regressions: 1676840
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.