Closed
Bug 748764
(CVE-2012-1942)
Opened 13 years ago
Closed 13 years ago
Possible Arbitrary Code Execution by Update Service
Categories
(Toolkit :: Application Update, defect)
Tracking
()
VERIFIED
FIXED
mozilla15
People
(Reporter: curtisk, Assigned: bbondy)
References
Details
(Keywords: reporter-external, sec-critical, Whiteboard: [sg:critical][advisory-tracking+])
Attachments
(1 file, 2 obsolete files)
2.39 KB,
patch
|
robert.strong.bugs
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Hi, I notice two security issues in the new Mozilla updater service which came with FF12 on Windows I would like to report. a) The service path is not quoted which means you can end up with the potential for a planting attack (e.g. if you can create C:\Program.exe it will run it when starting the service). b) The service itself can be tricked into loading a DLL into the process with SYSTEM privileges and it will run its DLLMain (meaning you can do anything you like). You call LoadLibrary instead of LoadLibraryEx on line 329 of workmonitor.cpp when checking that you have the correct resource string. It should be called to only load it as a resource. The following C# code will run the service and will load a DLL as long as the files C:\sourcecode\exploitme.dll and C:\sourcecode\updater.exe are the same. The DLL can then in DllMain run something like CreateProcess. using System; using System.ServiceProcess; namespace FFService { class Program { static void Main(string[] args) { try { ServiceController c = new ServiceController("MozillaMaintenance"); c.Start(new string[] { "aaa", "software-update", "C:\\sourcecode\\exploitme.dll", "aaaa", "c:\\sourcecode\\", "cccc" }); } catch (Exception e) { Console.WriteLine(e); } } } } I have only tested in XP Pro SP3 but I assume it will work on Win7 and Vista just the same. If you need more information let me know. James Forshaw Principal Security Consultant Context Information Security Registered no: 3574635 | Certified to ISO/IEC 27001:2005 (BSI Certificate IS 553326) and ISO 9001:2008 (BSI Certificate FS 581360) www.contextis.com | 30 Marsh Wall, London E14 9TP, UK | Mob: +44 (0)7827 248760 | Tel: 0207 537 7515 | Fax: 0207 537 1071 The information contained in this email and any attachments may be legally privileged and confidential. If you are not the intended recipient you are notified that disclosing, copying, distributing or taking any action in reliance on the contents of this information is strictly prohibited. If you are not the intended recipient please contact us immediately. Any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the company.
Assignee | ||
Comment 1•13 years ago
|
||
Thanks for the report.
Assignee: nobody → netzen
Attachment #618330 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 2•13 years ago
|
||
(Removed the WinUtils.cpp from my local patch by the way, that was accidentally included)
![]() |
||
Comment 3•13 years ago
|
||
Comment on attachment 618330 [details] [diff] [review] Patch v1. Brian, do the other security measures prevent this from being exploitable?
Attachment #618330 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 4•13 years ago
|
||
For a) no but the user would need write access into C:\ directly. For b) yes we verify that the updater.exe is the same as the one in the installation directory before loading the library to make sure it is an actual updater. So both attacks from what I understand require write access to elevated directories.
![]() |
Reporter | |
Comment 5•13 years ago
|
||
So anyone running as admin (which is just about anyone on windows) would have rights to these directories I think. On vista and 7 they should get warnings when trying to write to some directories as that may require a UAC decision depending on the settings the user has.
![]() |
||
Comment 6•13 years ago
|
||
Agreed and I am fairly certain that anyone running as admin that is tricked into setting this up could as easily be tricked into just executing what this would accomplish with the same end result without jumping through as many hoops.
Assignee | ||
Comment 7•13 years ago
|
||
so I think the main worry is that an admin could execute code as the system account. But any admin can already do this very easily by simply creating a new service that runs as the system account and then starting it.
![]() |
Reporter | |
Comment 8•13 years ago
|
||
I thought users were default admin on XP/Vista/7 if there were the first and/or only user of the system? So this is still a large segment of our user population. I do not mean to discount comment 6 but if users are default for admin rights we may be raising the stakes just a bit for attack surface no?
Assignee | ||
Comment 9•13 years ago
|
||
Just a note that on Windows XP the code in question does not execute. Also if UAC is off the code does not execute. So I believe that writing to those locations would always have a UAC prompt for every case that the code executes.
Comment 10•13 years ago
|
||
You do not need to write to c: all paths are attacker supplied and thus controlled I can provider a proper poc tomorrow or compile that c# and change the paths and see where it tries to go using procmon etc
Comment 11•13 years ago
|
||
Sorry I only meant b) for a) you would need to write to c: but I have seen machines configured to allow you
Assignee | ||
Comment 12•13 years ago
|
||
Ya for b) I see now. The apply to directory is an argument that is supplied. We do verify that this is a Firefox installation directory but that is in the signature check which happens after this LoadLibrary call. The user would have to pass a command line argument to the apply to directory that has a copy of the dll they want to load named updater.exe.
Comment 13•13 years ago
|
||
That is correct. I also tested Uac admin on win7 with my poc and it works on there to. Of course even if you directly checked for updater.exe in program files you would still have a race condition where the attacker could change the user supplied file before loadlibrary. I didn't look much further as I had other things but do you ensure that the file you signature check is the same file you exec? By for example copying to a secure location before verification? Or do you run the installed updater in the end? If not there might be a race there as well.
Assignee | ||
Comment 14•13 years ago
|
||
I think there is a potential for a race condition there, I added this patch to lock the file for write access before any of the checks happen.
Attachment #618401 -
Flags: review?(robert.bugzilla)
Comment 15•13 years ago
|
||
Also worth pointing out that with a), while yes on a vanilla Windows box it should not be possible to write a file to either C:\ or C:\program files, few people have vanilla configurations. I have seen users, admins and OEMs break these security principles before now. You can only really be reasonably confident in the directories you yourself created, not anything else on the file system. On a related note, if FF is installed to a non-standard directory does the updater service get installed to a secure directory elsewhere, or does it not get installed at all, or does it get installed along side?
Assignee | ||
Comment 16•13 years ago
|
||
If installed it will always be installed into program files. It will only be used if the user does not have write access into the apply to directory.
Comment 17•13 years ago
|
||
That fix for the race is not quite correct, for at least two reasons. 1) The attacker need not provide a path to a file system which honors the write locking. For example if he pointed it to a UNC path then he could change it on the remote server and Windows *could* return the new data at LoadLibrary time. 2) The semantics of LoadLibrary differ from CreateFile when it comes to paths. For example there is no checking done to make the path absolute so if you provide a relative path then LoadLibrary "could" search the path first (an unlikely condition). But probably more importantly, if you supply a filename with no extension then LoadLibrary always appends .DLL to the filename before loading, CreateFile will open the name as is.
Assignee | ||
Comment 18•13 years ago
|
||
I moved the discussion for Comment 14 - Comment 17 and the second patch to this new bug: Bug 748948
Assignee | ||
Updated•13 years ago
|
Attachment #618401 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•13 years ago
|
Attachment #618401 -
Attachment is obsolete: true
Updated•13 years ago
|
status-firefox-esr10:
--- → unaffected
status-firefox12:
--- → affected
status-firefox13:
--- → affected
status-firefox14:
--- → affected
status-firefox15:
--- → affected
tracking-firefox13:
--- → +
tracking-firefox14:
--- → +
tracking-firefox15:
--- → +
Assignee | ||
Comment 20•13 years ago
|
||
Last was r+ but changed patch enough to be more simplified. It does the same thing overall.
Attachment #618330 -
Attachment is obsolete: true
Attachment #619188 -
Flags: review?(robert.bugzilla)
![]() |
||
Updated•13 years ago
|
Attachment #619188 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 21•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/c9e68a3cb207
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
Assignee | ||
Comment 22•13 years ago
|
||
Comment on attachment 619188 [details] [diff] [review] Patch v2. [Approval Request Comment] Regression caused by (bug #): 481815 User impact if declined: unelevated process can run code elevated Testing completed (on m-c, etc.): I tested updates on elm, I will wait for 2 days on m-c to ensure updates are working there before pushing to aurora and beta. Risk to taking this patch (and alternatives if risky): low String changes made by this patch: none
Attachment #619188 -
Flags: approval-mozilla-beta?
Attachment #619188 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 23•13 years ago
|
||
Thank you for the reports and help James
Comment 24•13 years ago
|
||
Is there a compiled PoC anywhere that can be used for this. How do use the code snippet in comment 0 to see the bug isn't entirely clear to me since I need dlls to use from the C:\sourcecode directory (?), which are copied from somewhere? The updater?
Assignee | ||
Comment 25•13 years ago
|
||
You can just change the command line parameters to specify whichever directory you want. Comment 0 shows a C# program that you would run to reproduce, you can create a Dll with code that runs in DllMain I believe to reproduce.
Comment 26•13 years ago
|
||
Extract the PoC and run it on the command line with the mozilla service installed. updater.exe is just a renamed DLL.
Comment 27•13 years ago
|
||
Verified fixed in nightly trunk on XP SP3. Running with Firefox 12 and the compiled PoC, calc.exe is run when the PoC is invoked (and running as System). With the nightly, this no longer occurs.
Status: RESOLVED → VERIFIED
Comment 28•13 years ago
|
||
Comment on attachment 619188 [details] [diff] [review] Patch v2. [Triage Comment] Looks good on m-c, approving this sg:crit for Aurora 14 and Beta 13.
Attachment #619188 -
Flags: approval-mozilla-beta?
Attachment #619188 -
Flags: approval-mozilla-beta+
Attachment #619188 -
Flags: approval-mozilla-aurora?
Attachment #619188 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 29•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-aurora/rev/43265ac8befd http://hg.mozilla.org/releases/mozilla-beta/rev/b906d4ad6124
Comment 30•13 years ago
|
||
I'm not entirely convinced this is "critical" for today's Firefox users, because as a practical matter if you've got something running locally that can create c:\Program.exe you can probably trick that user into approving a UAC prompt and then do what you want. But fear of this kind of bug is exactly why we cautiously have chosen not to use the service for users with a limited account. Had this not been reported it would become critical when we enable that in the future; we might as well treat it as such now.
Updated•12 years ago
|
Whiteboard: [sg:critical] → [sg:critical][advisory-tracking+]
Updated•12 years ago
|
Alias: CVE-2012-1942
Updated•12 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
•