Security flaws in exploit code for MS03-007

• Previous message: Russ: "Re: Seti@home information leakage and remote compromise"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] [ attachment ]

Date:         Tue, 8 Apr 2003 00:51:50 +0100
From: Anony Mous <Anon21h@YAHOO.CO.UK>
To: NTBUGTRAQ@LISTSERV.NTBUGTRAQ.COM

All,

This is my first post to this list so let me briefly introduce myself. I
am young computer scientist (19 yrs old), mainly interested in
programming language design. I subscribed to the list in the hope of
increasing my understanding of security problems and finding ways to
solve them at the language design level. Whenever someone posts I ask
"how can language design alleviate/prevent this problem?" and try to
come up with answers. If anybody has ideas/info in this area please feel
free to share them with me.

Now then, in a recent post with the subject "Re: New attack vectors and
a vulnerability dissection of MS03-007" a link to exploit code for
RegEdit vulnerabilities on Windows 2000 was given:

http://packetstorm.acm.miami.edu/0304-exploits/regexploit.c

Introduction
------------
First off, thanks to the Packetstorm folks for sharing the code. I know
it's not meant to be "production code" but I did a quick audit and found
a number of dangerous practices that lead to exploits in the first
place. Some may already know about secure coding practices but for those
who don't, here is what I came up with. (I will dissect and explain why
some chunks of code are unsafe). I have posted this in the hope that all
programmers on this list may learn something to use in "security by
prevention" rather than "security by cure".

Forgetting to check for errors
------------------------------
Any code dealing with malloc and its cousins when working with buffers
needs to be checked. In the function ToWideChar(), at the beginning of
the exploit code we can see where failing to do this may lead to problems.

   nBufSize = MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, cszANSIstring,
-1, NULL, 0 );
   wideString = (WCHAR *)malloc(nBufSize +1);
   MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, cszANSIstring, -1,
wideString, nBufSize);

The above code is taken from the last few lines of ToWideChar(). The
variable "wideString" was declared as a "WCHAR" and is 16 bits long. If
the first call to MultiByteToWideChar() fails, nBufSize will be zero. On
the line that follows, malloc() would then allocate
nBufSize + 1 = 0 + 1 = 1 byte. malloc() returns a void pointer, hence
allowing it to be cast to any other pointer type. In this case a pointer
to a single byte is cast to a pointer to two bytes (remember wideString
was declared as WCHAR which is 16 bits or 2 bytes and hence WCHAR *, is
a pointer to two bytes.) From that point on, any writes to wideString,
would write 2 bytes at a time and overflow that one byte buffer that was
allocated. Thankfully in this exploit code MultiByteToWideChar() doesn't
write anything when nBufSize = 0.

Another example is in the main() function:

   MastaBuff = (char *) LocalAlloc (LPTR,270); // rempli la
premiere partie
   MastaBuff[0] = '"'; memset (&MastaBuff[1],'0',260); // avec des zeros

Here again, if LocalAlloc() fails, the buffer isn't allocated and
something nasty happens. memset() would then go to overwrite space
assumed to belong to MastaBuff[]. What's worse, if the MastaBuff[] had
been allocated on the stack (it isn't in this case), a stack overflow
would result and this tends to be worse than a heap overflow.

Lessons learnt:
-always check for errors when dealing with return values that you then
use to allocate buffers
-watch your casts from void pointers to pointers of other types

"Signedness" related errors
---------------------------
I have come to find that, for any given function that takes a signed
integer parameter of any size (8,16,32 bit etc.), if that parameter is
used with malloc() (or its cousins) in allocating memory, the function
is susceptible to buffer overflows. This also holds true for any
function that writes to a buffer, and is given the size of the buffer
through an unsigned integer parameter.

 From the exploit code, the Write() function is defined as:

   void Write (const char *str, int number)

where the "number" parameter is a signed integer. This parameter is then
used in a call to the WriteFile() function to specify how many bytes to
write to the registry file:

   WriteFile (RegFile,str,number,&lpNumberOfBytesWritten,NULL);

So what's the problem? Well, the WriteFile() function is defined as:

   BOOL WriteFile(
     HANDLE hFile,
     LPCVOID lpBuffer,
     DWORD nNumberOfBytesToWrite,
     LPDWORD lpNumberOfBytesWritten,
     LPOVERLAPPED lpOverlapped
   );

You will notice the third parameter nNumberOfBytesToWrite, is a DWORD.
The Windows Platform SDK documentation says that a DWORD is a 32-bit
*unsigned* integer. So in the call:

  WriteFile (RegFile,str,number,&lpNumberOfBytesWritten,NULL);

the signed integer parameter "number" is passed as an argument to the
unsigned integer parameter nNumberOfBytesToWrite. This results in an
automatic conversion from signed to unsigned integer. If number = 3, 3
bytes are written. If number = 400, then 400 bytes are written. However,
if someone could call the function Write() and pass -1 for the number of
bytes to write to the registry file, something nasty happens. When you
convert -1 to unsigned, you get 4,294,967,295. This means WriteFile()
attempts to write over 4 Gigabytes of data to the registry file!! This
will not only overflow "str" which would be much smaller than that, but
it will also lead to a denial of service attack as the OS tries to
satisfy that request. On a desktop, the machine will probably crash, on
a server precious CPU cycles would be wasted if it doesn't crash.

Lessons learned:
-use unsigned types when dealing with buffers. All(?) memory functions
expected unsigned integers.
-if you need to use signed types, then check all function parameters for
negative values before using them
-always check the size and signedness of aliased data types (e.g. DWORD,
LPDWORD etc.) before using them.

Unchecked user input
--------------------
A common security error is to fail to validate user input. The error may
be as simple as not checking the length of the input. In the main()
function we have:

   myurl = (char *) LocalAlloc (LPTR, strlen (argv[1])+10);
   lstrcpy (myurl,argv[1]);

   for (i=0; i < strlen (argv[1]); argv[1][i++]^=0x99); // encrypte l'URL
   lstrcat (RealGenericShellcode,argv[1]); // creation du shellcode final

The call to LocalAlloc() is used to allocate a buffer of size strlen
(argv[1])+10, but no check is made to ensure success. Since argv[1] can
be provided by the user, at the console someone can do:

  C:\exploit\Debug> type SomeMassiveFile.txt > exploit.exe

where exploit.exe is the exe from the posted exploit code and
SomeMassiveFile.txt is a sufficiently large file. This would likely
cause LocalAlloc to fail, then lstrcpy() would overwrite whatever
happens to be pointed to by myurl. Then in the "for" loop, either
strlen() overflows if the input's length can't fit in an unsigned
integer (the return type of strlen()) but since this is unlikely, you
get the long running loop eating away at CPU cycles.

Lessons learnt:
-do validation checks on user input (don't use the "accept everything
except" principle but rather the "deny everything except" principle).
-always check for memory allocation failures

Conclusion
-----------
The exploit code is only about 115 lines (including blank lines) but
contains numerous security mistakes. C, although securable, is not
exactly the safest language to program in and great care is needed to
ensure secure code is produced.

Now to make this a "proper" post, here is a buffer overflow I have found
in the nslookup utility. It is sitting there just waiting for someone to
find an attack vector:

1. at the console, type "nslookup". (program is under the
\WINNT\system32 folder). You get something like:

Default Server: foo.bar.co.uk
Address: 99.33.194.112

>_

2. at the nslookup prompt enter this:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

3. It seems to run and produce something like this:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Default Server: foo.bar.co.uk
Address: 99.33.194.112

then crashes with the message "The instruction at 0x01007dee referenced
memory at 0x6166161. The memory could not be read", which is a good
indication of a buffer overflow. This was tested on:
Windows 2000, Build 2195, SP3. nslookup version is 5.0.2195.4985 and its
language version is "English (United States)". Can anyone else reproduce
this? One possible use for this, since nslookup doesn't seem to check
input length, is to feed large amounts of input that clogs up the
network as its sent to the DNS server. If the DNS server doesn't deal
with that properly, we have another exploit opportunity there.

Cheers,Eric.

oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
Delivery co-sponsored by Prometric - More than testing, learning.
oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo
http://www.prometric.com

Prometric, part of The Thomson Corporation, is the leader in
technology-enabled testing and assessment services for information
technology certification, academic admissions, professional licensure and
certifications, computer-based driver's licensing, and corporate testing.

oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo

• Previous message: Russ: "Re: Seti@home information leakage and remote compromise"
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] [ attachment ]