Re: [PATCH] winecfg: Fix crash caused by calling set_reg_key with NULL value

[Evan Stade]

On 8/3/07, Nigel Liang  wrote:
> diff --git a/programs/winecfg/winecfg.c b/programs/winecfg/winecfg.c
> index 84dd4c8..05f322c 100644
> --- a/programs/winecfg/winecfg.c
> +++ b/programs/winecfg/winecfg.c
> @@ -468,11 +468,15 @@ void set_reg_key(HKEY root, const char *
>  wpath = HeapAlloc(GetProcessHeap(), 0, (strlen(path)+1)*sizeof(WCHAR));
>  wname = HeapAlloc(GetProcessHeap(), 0, (strlen(name)+1)*sizeof(WCHAR));
> -
>
> wvalue = HeapAlloc(GetProcessHeap(), 0, (strlen(value)+1)*sizeof(WCHAR));
>
> MultiByteToWideChar(CP_ACP, 0, path, -1, wpath, strlen(path)+1);
> MultiByteToWideChar(CP_ACP, 0, name, -1, wname, strlen(name)+1);
> -
>
> MultiByteToWideChar(CP_ACP, 0, value, -1, wvalue, strlen(value)+1);
> -
> -
>
> if (value)
> -
>
> {
> -
>    wvalue = HeapAlloc(GetProcessHeap(), 0, (strlen(value)+1)*sizeof(WCHAR));
> -
>    MultiByteToWideChar(CP_ACP, 0, value, -1, wvalue, strlen(value)+1);
> -
>
> }
>
> set_reg_key_ex(root, wpath, wname, wvalue, REG_SZ);
>
> --
> 1.4.1

You need to initialize wvalue to NULL

[PATCH] winecfg: Fix crash caused by calling set_reg_key with NULL value

[Nigel Liang]

On 8/3/07, Evan Stade  wrote:
> On 8/3/07, Nigel Liang  wrote:
> > diff --git a/programs/winecfg/winecfg.c b/programs/winecfg/winecfg.c
> > index 84dd4c8..05f322c 100644
> > --- a/programs/winecfg/winecfg.c
> > +++ b/programs/winecfg/winecfg.c
> > @@ -468,11 +468,15 @@ void set_reg_key(HKEY root, const char *
> >  wpath = HeapAlloc(GetProcessHeap(), 0, (strlen(path)+1)*sizeof(WCHAR));
> >  wname = HeapAlloc(GetProcessHeap(), 0, (strlen(name)+1)*sizeof(WCHAR));
> > -
> >
> > wvalue = HeapAlloc(GetProcessHeap(), 0, (strlen(value)+1)*sizeof(WCHAR));
> >
> > MultiByteToWideChar(CP_ACP, 0, path, -1, wpath, strlen(path)+1);
> > MultiByteToWideChar(CP_ACP, 0, name, -1, wname, strlen(name)+1);
> > -
> >
> > MultiByteToWideChar(CP_ACP, 0, value, -1, wvalue, strlen(value)+1);
> > -
> > -
> >
> > if (value)
> > -
> >
> > {
> > -
> >    wvalue = HeapAlloc(GetProcessHeap(), 0, (strlen(value)+1)*sizeof(WCHAR));
> > -
> >    MultiByteToWideChar(CP_ACP, 0, value, -1, wvalue, strlen(value)+1);
> > -
> >
> > }
> >
> > set_reg_key_ex(root, wpath, wname, wvalue, REG_SZ);
> >
> > --
> > 1.4.1
>
> You need to initialize wvalue to NULL
>
> --
> Evan Stade

Why is that? The code is not dependent on wvalue being NULL, but it
checks "value" to catch NULL pointers passed in from the caller. I
think you may have missed that... ;-)

-Nigel

[PATCH] winecfg: Fix crash caused by calling set_reg_key with NULL value

[Evan Stade]

On 8/3/07, Nigel Liang  wrote:
> On 8/3/07, Evan Stade  wrote:
> > On 8/3/07, Nigel Liang  wrote:
> > > diff --git a/programs/winecfg/winecfg.c b/programs/winecfg/winecfg.c
> > > index 84dd4c8..05f322c 100644
> > > --- a/programs/winecfg/winecfg.c
> > > +++ b/programs/winecfg/winecfg.c
> > > @@ -468,11 +468,15 @@ void set_reg_key(HKEY root, const char *
> > >  wpath = HeapAlloc(GetProcessHeap(), 0, (strlen(path)+1)*sizeof(WCHAR));
> > >  wname = HeapAlloc(GetProcessHeap(), 0, (strlen(name)+1)*sizeof(WCHAR));
> > > -
> > >
> > > wvalue = HeapAlloc(GetProcessHeap(), 0, (strlen(value)+1)*sizeof(WCHAR));
> > >
> > > MultiByteToWideChar(CP_ACP, 0, path, -1, wpath, strlen(path)+1);
> > > MultiByteToWideChar(CP_ACP, 0, name, -1, wname, strlen(name)+1);
> > > -
> > >
> > > MultiByteToWideChar(CP_ACP, 0, value, -1, wvalue, strlen(value)+1);
> > > -
> > > -
> > >
> > > if (value)
> > > -
> > >
> > > {
> > > -
> > >    wvalue = HeapAlloc(GetProcessHeap(), 0, (strlen(value)+1)*sizeof(WCHAR));
> > > -
> > >    MultiByteToWideChar(CP_ACP, 0, value, -1, wvalue, strlen(value)+1);
> > > -
> > >
> > > }
> > >
> > > set_reg_key_ex(root, wpath, wname, wvalue, REG_SZ);
> > >
> > > --
> > > 1.4.1
> >
> > You need to initialize wvalue to NULL
> >
> > --
> > Evan Stade
>
> Why is that? The code is not dependent on wvalue being NULL, but it
> checks "value" to catch NULL pointers passed in from the caller. I
> think you may have missed that... ;-)
>
> -Nigel

The code depends on wvalue either being a valid pointer or NULL, and
if value is NULL then you set wvalue to neither.  It should also give
a compiler warning when you compile because you are using an
uninitialized variable. get_reg_key does make these initializations by
the way.