Followup: vuln in WinBlox monitor for winnt

["Oliver Lavery" <oliver.lavery () sympatico ca>]

        The good fellows that moderate this list pointed out to me that my
last post regarding Liu Die Yu's winblox utility was a little thin on
details and might get percieved as a bit of a pissing competition. That's
precisely what I was trying to avoid by being vague, so let's get this
clear=85

        WinBlox is using the App_InitDLLs registry key to install a wrapper
around the CreateFileW function in _every_ process system wide. This isn't a
new trick, it's a simple version of the way many user-mode rootkits on NT
work. And I know first hand that several readers on this list have thought
about the same things Liu is doing in the past.

        The problem with these system wide hooking tricks is that they are
very powerful, but also very risky. When you change the behaviour of a
critical windows function like CreateFileW you run the risk of a bug in your
code introducing a new vuln into every single process on the system. So it's
of paramount importance that the wrapper code be bullet-proof. Taking a look
at Liu's code, it's anything but bulletproof, the execution path = following
from every call to CreateFileW is full of nasty programming practices that
can create vulns.

        Here's a full back-of-the-napkin analysis of the dangerous _sprintf
call I mentioned in the previous post (you'll find the disassembly listing
there), it's hypothetical and just off the top of my head, but the point is
the potential implications of buggy code when your trying to make this sort
of patch, not the strength of this specific analysis:

> Afaict, he's calling _sprintf to print data into a 2k stack based 
> buffer, called Text in the disassembly (ebp - 0x800). Look at the 
> format string:
>
> "CreateFile:%s > %s ==> %s --> %s"
>
>       So in order to trigger a buffer overflow in any process on a system 
> where Liu's runtime patch is running, you need to get the length of 
> this string, with substitutions, to exceed 2k in any process locally 
> or remotely. Let's look at some of his sample output:
>
> 2004/03/26 @ 09:33:29.224 # createfile:c:\program 
> files\icqlite\icqlite.exe
> > "c:\program files\icqlite\icqlite.exe" -minimize ==>
> generic_read|generic_write --> 
> \\?\root#system#0000#{3e227e76-690d-11d2-8161-0000f8775bf1}\{cd171de3-
> 69e5-1
> 1d2-b56d-0000f8754380}&{9b365890-165f-11d0-a195-0020afd156e4}
>
> 2004/03/26 @ 09:33:29.144 # createfile:c:\windows\explorer.exe > 
> c:\windows\explorer.exe ==> unknown_access_code:00000000 --> 
> c:\windows\system32\wdmaud.drv
>
> 2004/03/26 @ 09:27:45.590 # 
> createfile:\??\c:\windows\system32\winlogon.exe
> > winlogon.exe ==> generic_read --> c:\windows\system32\kbdus.dll
>
>       The arguments in order are the name of the current process, the 
> current processes command line, a textual representation of the flags 
> to CreateFileW, and finally the file being opened. Getting this to 
> exceed 2k doesn't sound too tough to me; the only length check is on 
> the file name. Since the last substitution is the name of the file 
> which is user supplied in many cases (like IIS for example...), it's 
> very easy to stick a shellcode in there.
>
>       So what this thing is going to do is stick a potential buffer 
> overflow into every process in the system that can potentially be 
> triggered whenever a file is opened. Not so very good. If I'm correct 
> finding cases where it can be exploited is a bit tedious, since each 
> process that will be running Liu's patch is different, but that's not 
> the point. You don't want to 'fix' windows by injecting a potential 
> buffer-overflow into it whenever a file is opened. And there are 
> several other potentially dangerous _sprintfs that follow throughout 
> his new CreateFileW function.
>
>       Regardless of whether or not I've made a mistake in my analysis, 
> there's plenty of evidence that this thing is not quite ready for 
> prime-time, which is a shame, because it's a good idea.

        That's it. No pissing competition. Liu's onto something very good
here, but as anyone who installs MS patches will tell ya, you've got to see
the full implications of a fix before you choose to apply it. Until this
thing gets rewritten properly, and follows even the most basic principals of
secure coding, it'll cause more problems than it fixes, in my opinion.

        I firmly believe that these sorts of tricks have tonnes of potential
and are going to become even more common in the future of the "so called
security community" tho' ;)

Cheers,
~ol


---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.605 / Virus Database: 385 - Release Date: 01/03/2004