Fixed bug 3334 - SDL_ShowMessageBox uses wrong index and accesses un-allocated memory
authorSam Lantinga <slouken@libsdl.org>
Fri, 11 Aug 2017 19:42:39 -0700
changeset 11228af4c3dc6b97f
parent 11227 10b07421903d
child 11229 436b07ff41b4
Fixed bug 3334 - SDL_ShowMessageBox uses wrong index and accesses un-allocated memory

romain.lacroix

For the windows implementation of SDL_ShowMessageBox() : ./src/video/windows/SDL_windowsmessagebox.c:345 WIN_ShowMessageBox()

The implementation in 2.0.4 uses "button index" for parameter "id" of function AddDialogButton().

It then expects the value provided in param wParam of function MessageBoxDialogProc() to be a valid index of a button.

It uses this value to index in the array of buttons when DialogBoxIndirect() returns (line 474 : *buttonid = buttons[which].buttonid;)

However, when dismissing this box with Escape, the return value of DialogBoxIndirect will be SDL_MESSAGEBOX_BUTTON_ESCAPEKEY_DEFAULT (=2) which is not always a valid index of array buttons.

When the array buttons has a length less or equal than 2, the memory access is invalid; I can see that the value written to *buttonId is uninitialized memory (random value).

The fix I propose : use value "buttonid" (field of button) for parameter "id" of AddDialogButton(), then copy return value of DialogBoxIndirect() in *buttonid. This way, we will not use an out-of-bounds index in array buttons.
src/video/windows/SDL_windowsmessagebox.c
     1.1 --- a/src/video/windows/SDL_windowsmessagebox.c	Fri Aug 11 19:36:12 2017 -0700
     1.2 +++ b/src/video/windows/SDL_windowsmessagebox.c	Fri Aug 11 19:42:39 2017 -0700
     1.3 @@ -463,7 +463,7 @@
     1.4          } else {
     1.5              isDefault = SDL_FALSE;
     1.6          }
     1.7 -        if (!AddDialogButton(dialog, x, y, ButtonWidth, ButtonHeight, buttons[i].text, i, isDefault)) {
     1.8 +        if (!AddDialogButton(dialog, x, y, ButtonWidth, ButtonHeight, buttons[i].text, buttons[i].buttonid, isDefault)) {
     1.9              FreeDialogData(dialog);
    1.10              return -1;
    1.11          }
    1.12 @@ -476,8 +476,7 @@
    1.13          ParentWindow = ((SDL_WindowData*)messageboxdata->window->driverdata)->hwnd;
    1.14      }
    1.15  
    1.16 -    which = DialogBoxIndirect(NULL, (DLGTEMPLATE*)dialog->lpDialog, ParentWindow, (DLGPROC)MessageBoxDialogProc);
    1.17 -    *buttonid = buttons[which].buttonid;
    1.18 +    *buttonid = DialogBoxIndirect(NULL, (DLGTEMPLATE*)dialog->lpDialog, ParentWindow, (DLGPROC)MessageBoxDialogProc);
    1.19  
    1.20      FreeDialogData(dialog);
    1.21      return 0;