748764 - (CVE-2012-1942) Possible Arbitrary Code Execution by Update Service

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

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: [phone] | Tel:Â 0207 537 7515 | Fax: [phone]



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.

Comment 1

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Thanks for the report.

Comment 2

[Brian R. Bondy [:bbondy]]

(Removed the WinUtils.cpp from my local patch by the way, that was accidentally included)

Comment 3

[Robert Strong (they/them - no direct email)]

Comment on attachment 618330 [details] [diff] [review]
Patch v1.

Brian, do the other security measures prevent this from being exploitable?

Comment 4

[Brian R. Bondy [:bbondy]]

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.

Comment 5

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

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

[Robert Strong (they/them - no direct email)]

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.

Comment 7

[Brian R. Bondy [:bbondy]]

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.

Comment 8

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

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?

Comment 9

[Brian R. Bondy [:bbondy]]

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

[James Forshaw]

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

[James Forshaw]

Sorry I only meant b) for a) you would need to write to c: but I have seen machines configured to allow you

Comment 12

[Brian R. Bondy [:bbondy]]

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

[James Forshaw]

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.

Comment 14

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1. Race condition

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.

Comment 15

[James Forshaw]

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?

Comment 16

[Brian R. Bondy [:bbondy]]

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

[James Forshaw]

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.

Comment 18

[Brian R. Bondy [:bbondy]]

I moved the discussion for Comment 14 - Comment 17 and the second patch to this new bug: Bug 748948

Comment 20

[Brian R. Bondy [:bbondy]]

Attachment: Patch v2.

Last was r+ but changed patch enough to be more simplified.  It does the same thing overall.

Comment 21

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/mozilla-central/rev/c9e68a3cb207

Comment 22

[Brian R. Bondy [:bbondy]]

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

Comment 23

[Brian R. Bondy [:bbondy]]

Thank you for the reports and help James

Comment 24

[Al Billings [:abillings - ex-MoCo]]

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?

Comment 25

[Brian R. Bondy [:bbondy]]

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

[James Forshaw]

Attachment: Compiled PoC

Extract the PoC and run it on the command line with the mozilla service installed. updater.exe is just a renamed DLL.

Comment 27

[Al Billings [:abillings - ex-MoCo]]

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.

Comment 28

[Alex Keybl [:akeybl]]

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.

Comment 29

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/43265ac8befd
http://hg.mozilla.org/releases/mozilla-beta/rev/b906d4ad6124

Comment 30

[Daniel Veditz [:dveditz] (OOO: back Aug 26)]

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.

Comment 31

[Raymond Forbes[:rforbes]]

rforbes-bugspam-for-setting-that-bounty-flag-20130719