Re: winex11.drv: Fix buffer overflow bug in X11DRV_KeyEvent() andX11DRV_ToUnicodeEx()

[Dmitry Timoshkov]

"Muneyuki Noguchi" <nogu.dev(a)gmail.com> wrote:
> winex11.drv: Fix buffer overflow bug in X11DRV_KeyEvent() and
> X11DRV_ToUnicodeEx()
> +    Str = (char *)malloc(64);
> +    if (Str == NULL)
> +        ERR("Failed to allocate memory!\n");

Please don't use malloc() in Wine, use win32 Heap*** APIs instead.
Also, you need to properly handle memory allocation errors, not just
print an ERR.

-- 
Dmitry.

winex11.drv: Fix buffer overflow bug in X11DRV_KeyEvent() andX11DRV_ToUnicodeEx()

[Rob Shearman]

2008/8/24 Dmitry Timoshkov <dmitry(a)codeweavers.com>:
> "Muneyuki Noguchi" <nogu.dev(a)gmail.com> wrote:
> > winex11.drv: Fix buffer overflow bug in X11DRV_KeyEvent() and
> > X11DRV_ToUnicodeEx()
> > +    Str = (char *)malloc(64);
> > +    if (Str == NULL)
> > +        ERR("Failed to allocate memory!\n");
>
> Please don't use malloc() in Wine, use win32 Heap*** APIs instead.
> Also, you need to properly handle memory allocation errors, not just
> print an ERR.
> @@ -1352,7 +1353,7 @@ static void update_lock_state(BYTE vkey, WORD scan, DWORD time)
>  void X11DRV_KeyEvent( HWND hwnd, XEvent *xev )
>  {
>      XKeyEvent *event = &xev->xkey;
> -    char Str[24];
> +    char *Str;
>      KeySym keysym = 0;
>      WORD vkey = 0, bScan;
>      DWORD dwFlags;
> @@ -1364,19 +1365,29 @@ void X11DRV_KeyEvent( HWND hwnd, XEvent *xev )
>      TRACE_(key)("type %d, window %lx, state 0x%04x, keycode 0x%04x\n",
>       event->type, event->window, event->state, event->keycode);
>
> +    Str = (char *)malloc(64);
> +    if (Str == NULL)
> +        ERR("Failed to allocate memory!\n");
>      wine_tsx11_lock();
>      /* Clients should pass only KeyPress events to XmbLookupString */
>      if (xic && event->type == KeyPress)
> +    {
>          ascii_chars = XmbLookupString(xic, event, Str, sizeof(Str), &keysym, &status);

In addition to the comments Dmitry has, sizeof(Str) no longer makes
sense now that Str has been made into a pointer. You also need to
update the other call to XmbLookupString in the patch.

-- 
Rob Shearman