Beginning on October 25th, 2016, Persona will no longer be an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 848417 - (CVE-2013-0799) Mozilla Maintenance Service buffer overflow allowing privilege escalation
: Mozilla Maintenance Service buffer overflow allowing privilege escalation
: csectype-priv-escalation, sec-high
Product: Toolkit
Classification: Components
Component: Application Update (show other bugs)
: 19 Branch
: x86_64 Windows 7
: -- critical (vote)
: mozilla22
Assigned To: Stephen A Pohl [:spohl]
: Robert Strong [:rstrong] (use needinfo to contact me)
Depends on:
Blocks: bgupdates
  Show dependency treegraph
Reported: 2013-03-06 09:17 PST by Frédéric Hoguin
Modified: 2014-07-24 16:08 PDT (History)
13 users (show)
abillings: sec‑bounty+
ryanvm: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proof of Concept source code and executable (6.14 KB, application/java-archive)
2013-03-06 09:17 PST, Frédéric Hoguin
no flags Details
Wip (2.81 KB, patch)
2013-03-06 15:03 PST, Stephen A Pohl [:spohl]
robert.strong.bugs: feedback+
netzen: feedback+
Details | Diff | Splinter Review
Patch (2.93 KB, patch)
2013-03-06 22:06 PST, Stephen A Pohl [:spohl]
netzen: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
lukasblakk+bugs: approval‑mozilla‑esr17+
Details | Diff | Splinter Review

Description Frédéric Hoguin 2013-03-06 09:17:53 PST
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:            => 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.

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.
Comment 1 Stephen A Pohl [:spohl] 2013-03-06 15:03:38 PST
Created attachment 721917 [details] [diff] [review]

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.
Comment 2 Stephen A Pohl [:spohl] 2013-03-06 15:17:53 PST
(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.
Comment 3 Robert Strong [:rstrong] (use needinfo to contact me) 2013-03-06 18:18:10 PST
Comment on attachment 721917 [details] [diff] [review]

Let's get bbondy's input on this
Comment 4 Brian R. Bondy [:bbondy] 2013-03-06 19:01:05 PST
Comment on attachment 721917 [details] [diff] [review]

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.
Comment 5 Brian R. Bondy [:bbondy] 2013-03-06 19:02:23 PST
This is
Comment 6 Stephen A Pohl [:spohl] 2013-03-06 22:06:37 PST
Created attachment 722057 [details] [diff] [review]

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.
Comment 7 Brian R. Bondy [:bbondy] 2013-03-07 04:02:10 PST
Comment on attachment 722057 [details] [diff] [review]

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

Thanks for taking this bug. Looks great.
Please run through try before landing.
Comment 8 Stephen A Pohl [:spohl] 2013-03-07 11:09:01 PST
The patch built fine, the test slaves seem to be awfully backed up however:

I ran xpcshell tests yesterday on the wip patch and the tests were green:

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.
Comment 9 Robert Strong [:rstrong] (use needinfo to contact me) 2013-03-07 13:42:58 PST
Pushed to mozilla-inbound
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-03-07 21:12:35 PST
Comment 11 Al Billings [:abillings] 2013-03-11 15:16:02 PDT
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 for process.
Comment 13 Alex Keybl [:akeybl] 2013-03-11 15:55:54 PDT
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?
Comment 14 Robert Strong [:rstrong] (use needinfo to contact me) 2013-03-11 16:07:33 PDT
Comment on attachment 722057 [details] [diff] [review]

[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
Comment 15 Lukas Blakk [:lsblakk] use ?needinfo 2013-03-11 16:36:15 PDT
Low risk, lets cover all our bases then and land to branches asap.
Comment 17 Matt Wobensmith [:mwobensmith][:matt:] 2013-03-28 14:39:08 PDT
Based on conversation with Stephen Pohl, QA has decided that a manual verification of this bug fix is not required.

Note You need to log in before you can comment on or make changes to this bug.