Bug 848417 (CVE-2013-0799)

Mozilla Maintenance Service buffer overflow allowing privilege escalation

RESOLVED FIXED in Firefox 20

Status

()

Toolkit
Application Update
--
critical
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Frédéric Hoguin, Assigned: spohl)

Tracking

({csectype-priv-escalation, sec-high})

19 Branch
mozilla22
x86_64
Windows 7
csectype-priv-escalation, sec-high
Points:
---
Bug Flags:
sec-bounty +
in-testsuite ?

Firefox Tracking Flags

(firefox19 wontfix, firefox20+ fixed, firefox21+ fixed, firefox22+ fixed, firefox-esr1720+ fixed, b2g18 unaffected)

Details

(Whiteboard: [adv-main20+][adv-esr1705+])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 721765 [details]
Proof of Concept source code and executable

The Mozilla Maintenance Service is vulnerable to a stack-based buffer overflow, allowing an unprivileged user to escalate privileges to NT_AUTHORITY\SYSTEM.

The bug lies in workmonitor.cpp, in the ProcessSoftwareUpdateCommand function, at lines 307-308, where a fixed size array is passed to GetInstallationDir:

WCHAR installDir[MAX_PATH] = {L'\0'};
if (!GetInstallationDir(argc, argv, installDir)) {

GetInstallationDir, in turn, performs an unchecked copy of argv[2] into the destination buffer (here, installDir):

static BOOL
GetInstallationDir(int argcTmp, LPWSTR *argvTmp, WCHAR aResultDir[MAX_PATH])
{
  if (argcTmp < 2) {
    return FALSE;
  }
  wcscpy(aResultDir, argvTmp[2]);


Maintenanceservice.exe wrongly assumes that its arguments have a valid size. However, due to the way the maintenance service is configured, any unprivileged user can start it, with arbitrary arguments: this is why Firefox can use it to upgrade without triggering an UAC prompt.

In the ProcessSoftwareUpdateCommand stack layout, the installDir buffer is located 588 bytes before the security cookie used to protect against stack based buffer overflows. Therefore, an argv[2] greater than 588 bytes will trigger a process termination due to this protection, and an event will be added to the Windows' event log. The attached PoC triggers the bug with a 592 bytes long argv[2].

The event can later be viewed with Windows' event viewer, under the Windows logs > Application category:

Problem signature:
P1: maintenanceservice.exe => exe filename
P2: 19.0.0.4794            => exe version
P3: 511eba22               => exe timestamp
[...]
P7: 0000313b               => faulting offset from ImageBase, this is the instruction right after __security_check_cookie in ProcessSoftwareUpdateCommand
P8: c0000409               => Exception type: Stack buffer overflow

Although it may look like the bug is not exploitable due to the stack overflow protection, it is indeed possible to exploit this bug:
-The service can be launched as many times as needed: the attacker gets as many tries as needed.
-The security cookie (stack overflow protection) is predictable (see below).
-ASLR and DEP are useless (see below).

Stack overflow protection:
The stack overflow protection works by generating a 32 bits integer which is called the security cookie. It is supposed to be random, however it is not. It is obtained by XORing the result of GetSystemTimeAsFileTime, GetTickCount, QueryPerformanceCounter, the process ID, and the thread ID:
-An attacker can call the 3 functions just before launching the service, and the results will be nearly identical, as they are all time-based. Even if there is a difference, it'll affect only the lowest bits of the security cookie.
-The process ID is predictable, as is the thread ID. Furthermore, they are low values, never superior to 1,000,000, and as such will affect only the lowest bits of the security cookie.

This makes the real entropy of the security cookie 16 bits, low enough to make an attack practical.

ASLR and DEP:
DEP is defeated with a ROP payload.
ASLR is useless because the base addresses of some DLLs like ntdll and kernel32 are randomized at boot time, and don't change until next reboot. An attacker running can just call GetModuleHandle to get their base addresses, and adapt the payload.

This was tested on Windows 7 64 bits, and it should work on all versions of Windows.
Flags: sec-bounty?
(Assignee)

Comment 1

4 years ago
Created attachment 721917 [details] [diff] [review]
Wip

Uses wcsncpy instead of wcscpy. One thing I don't remember off-hand is whether stack allocated buffers are zero'd out automatically on Windows, or if we need to do a memset to guarantee that installDir[MAX_PATH + 1] is null.
Attachment #721917 - Flags: feedback?(robert.bugzilla)
(Assignee)

Comment 2

4 years ago
(In reply to Stephen Pohl [:spohl] from comment #1)
> One thing I don't remember off-hand is
> whether stack allocated buffers are zero'd out automatically on Windows, or
> if we need to do a memset to guarantee that installDir[MAX_PATH + 1] is null.

Sorry, I meant to say: "that installDir[MAX_PATH] is null" (since installDir has a size of MAX_PATH + 1).

If buffers on the stack aren't zero'd out automatically, it might be preferable to simply set installDir[MAX_PATH] to L'\0' instead of doing a memset. wcsncpy takes care of the characters between the end of the string and MAX_PATH - 1 by setting them to null.
Attachment #721765 - Attachment mime type: application/octet-stream → application/java-archive
Comment on attachment 721917 [details] [diff] [review]
Wip

Let's get bbondy's input on this
Attachment #721917 - Flags: feedback?(netzen)
Comment on attachment 721917 [details] [diff] [review]
Wip

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

Stack allocated arrays (and heap allocated arrays) are not zero'd out by default.
If you initialize an array though with less elements than are in the array, then the rest of the elements will be initialized with 0's. 
So you could use memset, ZeroMemory, ..., but just doing = { L'\0' } is fine.
Attachment #721917 - Flags: feedback?(netzen) → feedback+
This is
Blocks: 307181
Attachment #721917 - Flags: feedback?(robert.bugzilla) → feedback+
(Assignee)

Comment 6

4 years ago
Created attachment 722057 [details] [diff] [review]
Patch

In this patch we also initialize the updateStatusFilePath buffer to L'\0' in IsStatusApplying. I think this is ready for a review.

It might be good to mention that this is a case of local privilege escalation. I don't believe that this can be exploited remotely, unless an attacker has gained the ability to launch services on the remote system through other means.
Assignee: nobody → spohl.mozilla.bugs
Attachment #721917 - Attachment is obsolete: true
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #722057 - Flags: review?(netzen)
Comment on attachment 722057 [details] [diff] [review]
Patch

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

Thanks for taking this bug. Looks great.
Please run through try before landing.
Attachment #722057 - Flags: review?(netzen) → review+
(Assignee)

Comment 8

4 years ago
The patch built fine, the test slaves seem to be awfully backed up however:
https://tbpl.mozilla.org/?tree=Try&rev=9c1c61be98e0

I ran xpcshell tests yesterday on the wip patch and the tests were green:
https://tbpl.mozilla.org/?tree=Try&rev=c6301284b6af

I'd appreciate it if someone could take it from here and land the patch if the feedback from try is sufficient. I don't have push privileges to inbound or trunk.
Pushed to mozilla-inbound
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8db16b10212
https://hg.mozilla.org/mozilla-central/rev/c8db16b10212
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox22: --- → fixed
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Since this bug affects old versions going back quite a way and not just trunk, it should have received a security rating before checkin after getting sec-approval+ on the patch or should have been set sec-approval? before any code was checked in. 

We will have to backport this to older versions and may need to do it on all affected before the next release so we don't expose ourselves.

See https://wiki.mozilla.org/Security/Bug_Approval_Process for process.
status-firefox19: --- → affected
status-firefox20: --- → affected
status-firefox21: --- → affected
status-firefox-esr17: --- → affected
tracking-firefox20: --- → ?
tracking-firefox21: --- → ?
tracking-firefox22: --- → +
tracking-firefox-esr17: --- → ?
Flags: sec-bounty? → sec-bounty+
Keywords: sec-high
Keywords: csec-priv-escalation
From our read of the bug, this exploit requires an attacker to have access to the computer as an unprivileged user. We typically de-prioritize exploits requiring that the user already pwned a computer, unless the fix is low risk. Can we get approval nominations on this bug with a risk assessment asap?

Updated

4 years ago
tracking-firefox21: ? → +
Comment on attachment 722057 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 307181
User impact if declined: possible priv-escalation
Testing completed (on m-c, etc.): on m-c... there are already tests to make sure the maintenance service works.
Risk to taking this patch (and alternatives if risky): small risk since this has baked on m-c and there are tests that exercise the maintenance service
String or UUID changes made by this patch: None
Attachment #722057 - Flags: approval-mozilla-esr17?
Attachment #722057 - Flags: approval-mozilla-beta?
Attachment #722057 - Flags: approval-mozilla-aurora?
Attachment #722057 - Flags: approval-mozilla-esr17?
Attachment #722057 - Flags: approval-mozilla-esr17+
Attachment #722057 - Flags: approval-mozilla-beta?
Attachment #722057 - Flags: approval-mozilla-beta+
Attachment #722057 - Flags: approval-mozilla-aurora?
Attachment #722057 - Flags: approval-mozilla-aurora+
Low risk, lets cover all our bases then and land to branches asap.
tracking-firefox20: ? → +
tracking-firefox-esr17: ? → 20+
https://hg.mozilla.org/releases/mozilla-aurora/rev/1c57ac120964
https://hg.mozilla.org/releases/mozilla-beta/rev/5b9c7be62231
https://hg.mozilla.org/releases/mozilla-esr17/rev/937117c2028d
status-firefox19: affected → wontfix
status-firefox20: affected → fixed
status-firefox21: affected → fixed
status-firefox-esr17: affected → fixed
status-b2g18: --- → unaffected
Whiteboard: [adv-main20+][adv-esr1705+]
Alias: CVE-2013-0799
Based on conversation with Stephen Pohl, QA has decided that a manual verification of this bug fix is not required.
Group: core-security
You need to log in before you can comment on or make changes to this bug.