1690062 - (CVE-2021-29951) Windows Maintenance Service has a Weak DACL for Domain Networks

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P1)

Description

[James Forshaw]

Attachment: poc.cpp

Summary:

The Mozilla Maintenance service on Windows is configured to allow BUILTIN\Users SERVICE_START access which can be granted to normal remote users on domain networks allowing them to start the service remotely and pass arbitrary arguments to its service main.

Description:

Note, it’s not clear how much you care about this behavior, and perhaps it’s intentional? Up to you whether you fix it I guess, however I would highly recommend it.

In domain environments (less so in home networks) most users can authenticate to any other workstation or server. One commonly remotely exposed service is the Service Control Manager which manages the local services. A non-administrator can be granted access to a service as long as they can pass a series of access checks.

Basically when installing the maintenance service’s security descriptor in serviceinstall.cpp you grant BUILTIN\Users group SERVICE_START and SERVICE_STOP access. As this group is granted to all authenticated users including remote ones this means any remote user can start and stop the maintenance service. This is a risk as the start operation takes a command line which is used to influence the update operation, and has been a source of numerous bugs in the past. While there might not be any known bugs in the handling of arguments atm, is this something you want to risk? If such a bug was found then you could have SYSTEM RCE on domain networks.

Now, this isn’t quite as bad as it might be. For a start I’m guessing some enterprises don’t install the service and rely on their own installation mechanisms and this assumes the service is exposed remotely through the firewall on a host. Also since Windows 10 1709 a mitigation has been in place to prevent remote users not in the local admin group from getting access to certain SCM and service access rights. This is described in a support article. This blocks a normal user getting SERVICE_START access but this hasn’t been back ported to Windows 7 or 8.X which you still support so they would be vulnerable, and it's possible to disable the mitigation which some sysadmins might do.

My recommended fix is to use the NT AUTHORITY\INTERACTIVE group instead of the BUILTIN\Users. This group is only granted to interactive logins (which includes RDP logins), not network logins, which is presumably the behavior you want? Replacing the line in serviceinstall.cpp CreateWellKnownSid(WinBuiltinUsersSid, nullptr, sid, &SIDSize) with CreateWellKnownSid(WinInteractiveSid, nullptr, sid, &SIDSize) should be sufficient.

Proof of Concept:

I’ve provided a proof of concept as a C++ file which will access the local SCM over a TCP RPC connection which simulates remote access. It also disables the INTERACTIVE group from the caller’s token to ensure it’s not inadvertently getting access using that, this is needed as it’ll actually use the same token and not do a full network authentication but the same result has been tested remotely. You’ll need to put the C++ file into a project and compile it.

For reasons already outlined you’ll need to run this on Windows 7 or 8.1 as recent versions of Windows 10 have a mitigation to block this attack vector. Although if you want to test on Windows 10 read the previous link for the registry key settings to disable the mitigation.

1. Compile the C++ project.
2. Run the POC on Windows 7 or 8.1 with the maintenance service installed.
3. Observe the output, if it prints [SUCCESS] it should have been granted SERVICE_START access.

Expected Result:

The OpenService call fails with ERROR_ACCESS_DENIED.

Observed Result:

The OpenService call succeeds.

This bug is subject to a 90 day disclosure deadline. After 90 days elapse,
the bug report will become visible to the public. The scheduled disclosure
date is 2021-05-03. Disclosure at an earlier date is also possible if
agreed upon by all parties.

Comment 1

[Daniel Veditz [:dveditz]]

Rachel: who on your team is working on the maintenance service these days? This could be super bad if combined with another bug.

Comment 2

[Robin Steuber (she/her) [:bytesized]]

I'll look into it.

Comment 3

[Rachel Tublitz [:rachel]]

thank you Kirk!

Comment 4

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1690062 - r=agashlin

Comment 5

[Robin Steuber (she/her) [:bytesized]]

Attachment: Bug 1690062 - Test to ensure the Maintenance Service can only be started by interactive users r=agashlin

Depends on D105382

Comment 6

[Robin Steuber (she/her) [:bytesized]]

Even though this is not a sec-high or sec-critical bug, I'm going to wait to merge the tests. So let's keep this bug open.

Comment 7

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

https://hg.mozilla.org/integration/autoland/rev/89cb4083e3104c80036119edb4febd136de7091b
https://hg.mozilla.org/mozilla-central/rev/89cb4083e310

Comment 8

[James Forshaw]

What is the plan for when this fix will be released? As mentioned in the original report the 90 day deadline ends on 2021-05-03 where the issue will disclosed if a patch is not available. There is the option of a 14 day grace extension as long as the patch will be released within that time window.

Comment 9

[Robin Steuber (she/her) [:bytesized]]

The fix was landed in Firefox 87, which became the release version of Firefox on 2021-03-23.

Comment 10

[James Forshaw]

Great, I didn't notice anything in the release notes. Was this assigned a CVE? Also for interests sake at the same time I discovered that the mitigation Microsoft added to the SCM which I mentioned in the report was bypassable. They fixed the issue this month https://bugs.chromium.org/p/project-zero/issues/detail?id=2147.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

This shouldn't have been left open just for landing tests. We need to figure out what to do for ESR now since the next scheduled release doesn't go out until June 1.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

And we'll need to amend the Fx87 advisories once that gets sorted.

Kirk, please request ESR78 approval on this. It grafts cleanly as-landed.

Comment 13

[Daniel Veditz [:dveditz]]

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #6)

Even though this is not a sec-high or sec-critical bug, I'm going to wait to merge the tests. So let's keep this bug open.

Kirk: PLEASE don't do that. If the bug is fixed please mark it fixed, especially for a security bug. That will kick off our advisory and CVE assigning process, testing, and all the rest of the release stuff we need to do. Landing tests later for security bugs is important because people do look to turn those into exploits, so thank you for that! But for managing that later check-in after marking the bug fixed your two choices should be:

1. set the in-testsuite flag to "?" to indicate tests aren't landed yet. (reminder to yourself, or eventually someone will nag you about it)
2. clone this into a "land tests for bug 1690062" task bug.

I'll go with the in-testsuite? flag for now but if you want to clone a bug instead please do so and clear the flag here.

Note: we also need this fix on ESR 78

Comment 14

[Daniel Veditz [:dveditz]]

Tom: we'll need a CVE here and an update to the 87 advisories. We'll want to wait on pushing the advisory update until we know what we're doing with ESR 78 (push a mid-cycle release to meet the disclosure deadline? wait for current schedule?)

Comment 15

[Robin Steuber (she/her) [:bytesized]]

Comment on attachment 9203564 [details]
Bug 1690062 - r=agashlin

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: If unpatched, could be combined with other exploits to form an exploit chain.
• User impact if declined: If unpatched, could be combined with other exploits to form an exploit chain.
• Fix Landed on Version: 87
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is a bit risky because it deals with changing Windows permissions, which are complicated to get right, and can have serious implications if they are wrong. As far as I know, the only alternative is to leave this unpatched.
• String or UUID changes made by this patch:

Comment 16

[Robin Steuber (she/her) [:bytesized]]

(In reply to Daniel Veditz [:dveditz] from comment #13)

(In reply to Kirk Steuber (he/him) [:bytesized] from comment #6)

Even though this is not a sec-high or sec-critical bug, I'm going to wait to merge the tests. So let's keep this bug open.

Kirk: PLEASE don't do that. If the bug is fixed please mark it fixed, especially for a security bug. That will kick off our advisory and CVE assigning process, testing, and all the rest of the release stuff we need to do.

I wish I had known that before today. I read the Security Bug Approval Process document, and did my best to follow it. My apologies for getting it wrong.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9203564 [details]
Bug 1690062 - r=agashlin

Approved for 78.11esr. I'm also going to reset the ESR tracking flag in case we end up shipping this in a 78.10.1 dot release before then.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/9efd42e1e29d

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Fixed for 78.10.1esr also.
https://hg.mozilla.org/releases/mozilla-esr78/rev/cdde52e57ff2

Comment 20

[James Forshaw]

The Project Zero issue tracker will be opened today as the 90 days have expired (https://bugs.chromium.org/p/project-zero/issues/detail?id=2148) I'll mention it was fixed in FF87 and that it'll be fixed in 78.10.1esr. When is the ESR set for release?

Comment 21

[Daniel Veditz [:dveditz]]

We're shipping Wednesday or Thursday -- can we have a couple more days?

Comment 22

[James Forshaw]

If the final patch date will be definitely within the next 14 days, I can apply a grace extension. If that's the case here then I can apply it for your.

Comment 23

[Daniel Veditz [:dveditz]]

We will definitely be shipping the ESR fix this week.

Comment 24

[James Forshaw]

Thanks. I've applied the grace period to the deadline so the issue won't be opened today. It'll give until 2021/05/17.

Comment 25

[James Forshaw]

For info, as the fixes seems to have gone out I've opened the bug in our issues tracker. https://bugs.chromium.org/p/project-zero/issues/detail?id=2148

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55a4243e57c9
Test to ensure the Maintenance Service can only be started by interactive users r=agashlin

Comment 27

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/55a4243e57c9