Message-ID: <379844A798E94817B5F36B14F4A9F74F@celsius>
Date: Sun, 3 Nov 2013 22:07:09 +0100
From: "Stefan Kanthak" <stefan.kanthak@...go.de>
To: "Mario Vilas" <mvilas@...il.com>
Cc: full-disclosure@...ts.grok.org.uk, bugtraq <bugtraq@...urityfocus.com>
Subject: Re: Defense in depth -- the Microsoft way (part
	13): surprising and inconsistent behaviour, sloppy coding,
	sloppy QA, sloppy documentation

"Mario Vilas" <mvilas@...il.com> wrote:

> This may be a silly question, so I apologize in advance, but that would
> exactly be the advantage here? Using a NULL pointer is in most (if not all)
> those cases undocumented behavior to begin with. Unless I'm missing
> something, the problem is not so much with Win32 as it is with the C
> language in general...

For many of the Win32 functions I referenced here the behaviour of a NULL
pointer is but documented:

<http://msdn.microsoft.com/en-us/library/ms647492.aspx>

| If lpString is NULL, the function returns 0.

JFTR: I suspect that this is the reason why most of the other functions
      treat a NULL pointer as an empty string.

<http://msdn.microsoft.com/en-us/library/ms647487.aspx>
<http://msdn.microsoft.com/en-us/library/ms647490.aspx>
<http://msdn.microsoft.com/en-us/library/ms647491.aspx>

| When this function catches SEH errors, it returns NULL without null-
| terminating the string and without notifying the caller of the error.

<http://msdn.microsoft.com/en-us/library/ms682425.aspx>

| BOOL WINAPI CreateProcess(
|   _In_opt_     LPCTSTR lpApplicationName,
|   _Inout_opt_  LPTSTR lpCommandLine,

BOTH string arguments may be NULL!
Idem for the other CreateProcess*() functions.

<http://msdn.microsoft.com/en-us/library/ms645505.aspx>

| The dialog box title. If this parameter is NULL, the default title is Error.

<http://msdn.microsoft.com/en-us/library/aa364934.aspx>

| To determine the required buffer size, set this parameter to NULL and the
| nBufferLength parameter to 0.


Most of the Get*() functions return the required buffer size when the size
argument is 0 (or to small), but only
<http://msdn.microsoft.com/en-us/library/ms724301.aspx> explicitly says:

| If lpBuffer is NULL, this parameter must be zero.

and checks this contraint properly.


The problem is not the C language!
The problem is the inconsistent (and sloppy) implemenation of similar
functions of the Win32 API and their inconsistent and sloppy documentation.

regards
Stefan Kanthak


On Sun, Nov 3, 2013 at 4:30 PM, Stefan Kanthak <stefan.kanthak@...go.de>wrote:

> Hi @ll,
>
> the Win32 API is full of idiosyncrasies resp. surprising and inconsistent,
> poorly tested and documented behaviour.
>
> Just to pick one: NULL pointer as string argument.
>
> 0. lstrlen(NULL)
>    lstrcat(NULL, ...)      and  lstrcat(..., NULL)
>    lstrcmp(NULL, ...)      and  lstrcmp(..., NULL)
>    lstrcmpi(NULL, ...)     and  lstrcmpi(..., NULL)
>    lstrcpy(NULL, ...)      and  lstrcpy(..., NULL)
>    lstrcpyn(NULL, ..., 0)  and  lstrcpy(..., NULL, ...)
>
>    do not yield an exception, but treat their NULL arguments like an
>    empty string (when used as source), resp. return NULL (when used as
>    destination).
>
>
> 1. wsprintf(NULL, ...)       and  wvsprintf(NULL, ...)
>    wsprintf(..., NULL, ...)  and  wvsprintf(..., NULL, ...)
>
>    yield an access violation in USER32.DLL.
>
>
> 2. CommandLineToArgvW(NULL, ...)
>
>    yields an access violation in SHELL32.DLL.
>
>
> 3. CreateProcess(NULL, NULL, ...)
>    CreateProcessAsUser(..., NULL, NULL, ...)
>    CreateProcessWithLogonW(..., ..., ..., ..., NULL, NULL, ...)
>    CreateProcessWithTokenW(..., ..., NULL, NULL, ...)
>
>    yield an access violation in KERNEL32.DLL.
>
>
> 4. GetFileAttributes(NULL)
>
>    does not yield an exception, but treats the NULL argument like an
>    empty string.
>
>
> 5. GetBinaryType(NULL, ...)
>
>    does not yield an exception, but treats the NULL argument like an
>    empty string.
>
>
> 6. MessageBox(..., NULL, ...)  and  MessageBox(..., ..., NULL, ...)
>
>    do not yield an exception, but treat the NULL argument like an
>    empty string.
>
>
> 7. FatalAppExit(0, NULL)
>
>    does not yield an exception, but treats the NULL argument like an
>    empty string.
>
>
> 8. GetCurrentDirectory(..., NULL)
>
>    returns an error if the buffer size (the argument shown as ... here)
>    is sufficient to hold the result, else the required buffer size.
>
>    GetTempPath(..., NULL)
>    GetSystemDirectory(NULL, ...)
>    GetSystemWindowsDirectory(NULL, ...)
>    GetSystemWow64Directory(NULL, ...)
>    GetWindowsDirectory(NULL, ...)
>    GetComputerName(NULL, ...)
>
>    yield an access violation in NTDLL.DLL resp. KERNEL32.DLL if the
>    buffer size is sufficient to hold the result, else the required
>    buffer size.
>
>    GetUserName(NULL, ...)
>    GetComputerObjectName(..., NULL, ...)
>
>    do not yield an access violation, but return an error with
>    GetLastError() == ERROR_INSUFFICIENT_BUFFER.
>
>
> 9. GetUserName(NULL, NULL)
>    GetComputerName(NULL, NULL)
>
>    yield an access violation in KERNEL32.DLL.
>
>    GetComputerNameEx(..., NULL, NULL)
>    GetComputerObjectName(..., NULL, NULL)
>
>    do not yield an access violation, but return an error with
>    GetLastError() == ERROR_INVALID_PARAMETER.
>
>    JFTR: only the documentation of the last function (see
>          <http://msdn.microsoft.com/en-us/library/ms724301.aspx>)
>          explicitly says about the value of the third argument
>          "If lpBuffer is NULL, this parameter must be zero."
>          and checks this contraint properly.
>
>
> The expected behavior in all cases is but to return an error with
> GetLastError() == ERROR_INVALID_PARAMETER or similar.
>
>
> FIX: ALL interfaces of the Win32 API should^WMUST verify (ALL) their
>      arguments properly before using them and return an appropriate,
>      documented error code.
>
>
> stay tuned
> Stefan Kanthak

_______________________________________________
Full-Disclosure - We believe in it.
Charter: http://lists.grok.org.uk/full-disclosure-charter.html
Hosted and sponsored by Secunia - http://secunia.com/

Powered by blists - more mailing lists