Re: rpcrt4: fix NULL pointer access

[Gregory M. Turner]

On Thursday 20 November 2003 02:26 pm, Dimitrie O. Paun wrote:
> Ran into this while browsing the code, it seems to
> be a NULL pointer reference waiting to happen.
> Am I missing something?
>
> ChangeLog
>     Bail when we're out of memory.
> Index: dlls/rpcrt4/rpc_binding.c
>
> RCS file: /var/cvs/wine/dlls/rpcrt4/rpc_binding.c,v
> retrieving revision 1.15
> diff -u -r1.15 rpc_binding.c
> --- dlls/rpcrt4/rpc_binding.c	7 Oct 2003 22:54:17 -0000	1.15
> +++ dlls/rpcrt4/rpc_binding.c	20 Nov 2003 06:33:52 -0000
> @@ -516,7 +516,11 @@
>  {
>    DWORD len = strlen(dst), slen = strlen(src);
>    LPSTR ndst = HeapReAlloc(GetProcessHeap(), 0, dst,
> (len+slen+2)*sizeof(CHAR)); -  if (!ndst) HeapFree(GetProcessHeap(), 0,
> dst);
> - if (!ndst)
> - {
> - HeapFree(GetProcessHeap(), 0, dst);
> - return NULL;
> - }
> ndst[len] = ',';
> memcpy(ndst+len+1, src, slen*sizeof(CHAR));
> ndst[len+slen+1] = 0;
>
> @@ -527,7 +531,11 @@
>  {
>    DWORD len = strlenW(dst), slen = strlenW(src);
>    LPWSTR ndst = HeapReAlloc(GetProcessHeap(), 0, dst,
> (len+slen+2)*sizeof(WCHAR)); -  if (!ndst) HeapFree(GetProcessHeap(), 0,
> dst);
> - if (!ndst)
> - {
> - HeapFree(GetProcessHeap(), 0, dst);
> - return NULL;
> - }
> ndst[len] = ',';
> memcpy(ndst+len+1, src, slen*sizeof(WCHAR));
> ndst[len+slen+1] = 0;

looks like a real bug to me... but the consumers of those functions
probably assume success, so maybe the right solution is really to raise
an exception.  There are lots of places in rpcrt4 where exceptions ought
to raise on error conditions but don't, mainly due to developer laziness.
I guess it's hard to justify fixing error paths when success doesn't work
either ;)

So, at some point there needs to be an error-handling audit of rpcrt4;
until then, your patch is better than nothing, and probably should go in.