[PATCH 0/1] MR5392: win32u: Avoid writing past allocated memory in peek_message().
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/win32u/message.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/dlls/win32u/message.c b/dlls/win32u/message.c index 2b416f39241..5416587f254 100644 --- a/dlls/win32u/message.c +++ b/dlls/win32u/message.c @@ -522,7 +522,7 @@ BOOL set_keyboard_auto_repeat( BOOL enable ) * Unpack a message received from another process. */ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lparam, - void **buffer, size_t size ) + void **buffer, size_t size, size_t buffer_size ) { size_t minsize = 0; union packed_structs *ps = *buffer; @@ -585,7 +585,7 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa break; case WM_GETTEXT: case WM_ASKCBFORMATNAME: - if (!get_buffer_space( buffer, (*wparam * sizeof(WCHAR)), size )) return FALSE; + if (!get_buffer_space( buffer, (*wparam * sizeof(WCHAR)), buffer_size )) return FALSE; break; case WM_WININICHANGE: if (!*lparam) return TRUE; @@ -726,17 +726,17 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa minsize = sizeof(SCROLLINFO); break; case SBM_GETSCROLLINFO: - if (!get_buffer_space( buffer, sizeof(SCROLLINFO), size )) return FALSE; + if (!get_buffer_space( buffer, sizeof(SCROLLINFO), buffer_size )) return FALSE; break; case SBM_GETSCROLLBARINFO: - if (!get_buffer_space( buffer, sizeof(SCROLLBARINFO), size )) return FALSE; + if (!get_buffer_space( buffer, sizeof(SCROLLBARINFO), buffer_size )) return FALSE; break; case EM_GETSEL: case SBM_GETRANGE: case CB_GETEDITSEL: if (*wparam || *lparam) { - if (!get_buffer_space( buffer, 2 * sizeof(DWORD), size )) return FALSE; + if (!get_buffer_space( buffer, 2 * sizeof(DWORD), buffer_size )) return FALSE; if (*wparam) *wparam = (WPARAM)*buffer; if (*lparam) *lparam = (LPARAM)((DWORD *)*buffer + 1); } @@ -744,7 +744,7 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa case EM_GETRECT: case LB_GETITEMRECT: case CB_GETDROPPEDCONTROLRECT: - if (!get_buffer_space( buffer, sizeof(RECT), size )) return FALSE; + if (!get_buffer_space( buffer, sizeof(RECT), buffer_size )) return FALSE; break; case EM_SETRECT: case EM_SETRECTNP: @@ -755,7 +755,7 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa WORD *len_ptr, len; if (size < sizeof(WORD)) return FALSE; len = *(WORD *)*buffer; - if (!get_buffer_space( buffer, (len + 1) * sizeof(WCHAR), size )) return FALSE; + if (!get_buffer_space( buffer, (len + 1) * sizeof(WCHAR), buffer_size )) return FALSE; len_ptr = *buffer; len_ptr[0] = len_ptr[1] = len; *lparam = (LPARAM)(len_ptr + 1); @@ -780,26 +780,24 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa break; case CB_GETLBTEXT: { - size_t prev_size = size; if (combobox_has_strings( hwnd )) size = (send_message( hwnd, CB_GETLBTEXTLEN, *wparam, 0 ) + 1) * sizeof(WCHAR); else size = sizeof(ULONG_PTR); - if (!get_buffer_space( buffer, size, prev_size )) return FALSE; + if (!get_buffer_space( buffer, size, buffer_size )) return FALSE; break; } case LB_GETTEXT: { - size_t prev_size = size; if (listbox_has_strings( hwnd )) size = (send_message( hwnd, LB_GETTEXTLEN, *wparam, 0 ) + 1) * sizeof(WCHAR); else size = sizeof(ULONG_PTR); - if (!get_buffer_space( buffer, size, prev_size )) return FALSE; + if (!get_buffer_space( buffer, size, buffer_size )) return FALSE; break; } case LB_GETSELITEMS: - if (!get_buffer_space( buffer, *wparam * sizeof(UINT), size )) return FALSE; + if (!get_buffer_space( buffer, *wparam * sizeof(UINT), buffer_size )) return FALSE; break; case WM_NEXTMENU: { @@ -814,7 +812,7 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa case WM_SIZING: case WM_MOVING: minsize = sizeof(RECT); - if (!get_buffer_space( buffer, sizeof(RECT), size )) return FALSE; + if (!get_buffer_space( buffer, sizeof(RECT), buffer_size )) return FALSE; break; case WM_MDICREATE: { @@ -880,7 +878,7 @@ static BOOL unpack_message( HWND hwnd, UINT message, WPARAM *wparam, LPARAM *lpa } case WM_MDIGETACTIVE: if (!*lparam) return TRUE; - if (!get_buffer_space( buffer, sizeof(BOOL), size )) return FALSE; + if (!get_buffer_space( buffer, sizeof(BOOL), buffer_size )) return FALSE; break; case WM_DEVICECHANGE: if (!(*wparam & 0x8000)) return TRUE; @@ -2788,7 +2786,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) case MSG_NOTIFY: info.flags = ISMEX_NOTIFY; if (!unpack_message( info.msg.hwnd, info.msg.message, &info.msg.wParam, - &info.msg.lParam, &buffer, size )) + &info.msg.lParam, &buffer, size, buffer_size )) continue; break; case MSG_CALLBACK: @@ -2866,7 +2864,7 @@ int peek_message( MSG *msg, const struct peek_message_filter *filter ) case MSG_OTHER_PROCESS: info.flags = ISMEX_SEND; if (!unpack_message( info.msg.hwnd, info.msg.message, &info.msg.wParam, - &info.msg.lParam, &buffer, size )) + &info.msg.lParam, &buffer, size, buffer_size )) { /* ignore it */ reply_message( &info, 0, &info.msg ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5392
peek_message() peeks the messages from server in a loop, keeping the buffer and buffer_size from previous iteration. The message size returned from server for the message may be smaller than current buffer size. Then, unpack message gets the actual message size and may reallocate the buffer to actually smaller size. So while getting next message buffer_size used for server reply size may be bigger than actually allocated buffer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5392#note_65876
participants (2)
-
Paul Gofman -
Paul Gofman (＠gofman)