From 79a846d4862a1f6b00b537b47ad8352dbc3969c7 Mon Sep 17 00:00:00 2001 From: Sam Lantinga Date: Fri, 11 Aug 2017 19:42:39 -0700 Subject: [PATCH] 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 | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/video/windows/SDL_windowsmessagebox.c b/src/video/windows/SDL_windowsmessagebox.c index 9c21f4545335f..9b40a608fd898 100644 --- a/src/video/windows/SDL_windowsmessagebox.c +++ b/src/video/windows/SDL_windowsmessagebox.c @@ -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; } @@ -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;