Skip to content

Commit

Permalink
Fixed bug 3334 - SDL_ShowMessageBox uses wrong index and accesses un-…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
slouken committed Aug 12, 2017
1 parent 441d9ba commit 79a846d
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions src/video/windows/SDL_windowsmessagebox.c
Expand Up @@ -463,7 +463,7 @@ WIN_ShowMessageBox(const SDL_MessageBoxData *messageboxdata, int *buttonid)
} else {
isDefault = SDL_FALSE;
}
if (!AddDialogButton(dialog, x, y, ButtonWidth, ButtonHeight, buttons[i].text, i, isDefault)) {
if (!AddDialogButton(dialog, x, y, ButtonWidth, ButtonHeight, buttons[i].text, buttons[i].buttonid, isDefault)) {
FreeDialogData(dialog);
return -1;
}
Expand All @@ -476,8 +476,7 @@ WIN_ShowMessageBox(const SDL_MessageBoxData *messageboxdata, int *buttonid)
ParentWindow = ((SDL_WindowData*)messageboxdata->window->driverdata)->hwnd;
}

which = DialogBoxIndirect(NULL, (DLGTEMPLATE*)dialog->lpDialog, ParentWindow, (DLGPROC)MessageBoxDialogProc);
*buttonid = buttons[which].buttonid;
*buttonid = DialogBoxIndirect(NULL, (DLGTEMPLATE*)dialog->lpDialog, ParentWindow, (DLGPROC)MessageBoxDialogProc);

FreeDialogData(dialog);
return 0;
Expand Down

0 comments on commit 79a846d

Please sign in to comment.