[PATCH] user32: Avoid buffer overflow on long texts in winproc.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48559 Signed-off-by: Roman Pišl <rpisl(a)seznam.cz> --- dlls/user32/winproc.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/dlls/user32/winproc.c b/dlls/user32/winproc.c index 6de650cd1e..14555f5a71 100644 --- a/dlls/user32/winproc.c +++ b/dlls/user32/winproc.c @@ -517,16 +517,21 @@ LRESULT WINPROC_CallProcAtoW( winproc_callback_t callback, HWND hwnd, UINT msg, case CB_GETLBTEXT: if (lParam && WINPROC_TestLBForStr( hwnd, msg )) { - WCHAR buffer[512]; /* FIXME: fixed sized buffer */ + WCHAR *ptr, buffer[512]; + UINT msgGetTextLen = (msg == CB_GETLBTEXT) ? CB_GETLBTEXTLEN : LB_GETTEXTLEN; + LRESULT len = 0; - ret = callback( hwnd, msg, wParam, (LPARAM)buffer, result, arg ); + callback( hwnd, msgGetTextLen, wParam, 0, &len, arg ); + if (!(ptr = get_buffer( buffer, sizeof(buffer), (len + 1) * sizeof(WCHAR) ))) break; + ret = callback( hwnd, msg, wParam, (LPARAM)ptr, result, arg ); if (*result >= 0) { DWORD len; RtlUnicodeToMultiByteN( (LPSTR)lParam, ~0u, &len, - buffer, (strlenW(buffer) + 1) * sizeof(WCHAR) ); + ptr, (strlenW(ptr) + 1) * sizeof(WCHAR) ); *result = len - 1; } + free_buffer( buffer, ptr ); } else ret = callback( hwnd, msg, wParam, lParam, result, arg ); break; @@ -777,15 +782,20 @@ static LRESULT WINPROC_CallProcWtoA( winproc_callback_t callback, HWND hwnd, UIN case CB_GETLBTEXT: if (lParam && WINPROC_TestLBForStr( hwnd, msg )) { - char buffer[512]; /* FIXME: fixed sized buffer */ + char *ptr, buffer[512]; + UINT msgGetTextLen = (msg == CB_GETLBTEXT) ? CB_GETLBTEXTLEN : LB_GETTEXTLEN; + LRESULT len = 0; + callback( hwnd, msgGetTextLen, wParam, 0, &len, arg ); + if (!(ptr = get_buffer( buffer, sizeof(buffer), len + 1 ))) break; - ret = callback( hwnd, msg, wParam, (LPARAM)buffer, result, arg ); + ret = callback( hwnd, msg, wParam, (LPARAM)ptr, result, arg ); if (*result >= 0) { DWORD len; - RtlMultiByteToUnicodeN( (LPWSTR)lParam, ~0u, &len, buffer, strlen(buffer) + 1 ); + RtlMultiByteToUnicodeN( (LPWSTR)lParam, ~0u, &len, ptr, strlen(ptr) + 1 ); *result = len / sizeof(WCHAR) - 1; } + free_buffer( buffer, ptr ); } else ret = callback( hwnd, msg, wParam, lParam, result, arg ); break; -- 2.20.1
Hello, this one fell off the list. Is it completely wrong or just nobody is interested? Or writing a test would help (which is easy in this case)? Also simple controls demo applications are affected - pasting large text into the control is enough for crash. Roman Dne 06. 02. 20 v 18:33 Roman Pišl napsal(a):
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48559
Signed-off-by: Roman Pišl <rpisl(a)seznam.cz> --- dlls/user32/winproc.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/dlls/user32/winproc.c b/dlls/user32/winproc.c index 6de650cd1e..14555f5a71 100644 --- a/dlls/user32/winproc.c +++ b/dlls/user32/winproc.c @@ -517,16 +517,21 @@ LRESULT WINPROC_CallProcAtoW( winproc_callback_t callback, HWND hwnd, UINT msg, case CB_GETLBTEXT: if (lParam && WINPROC_TestLBForStr( hwnd, msg )) { - WCHAR buffer[512]; /* FIXME: fixed sized buffer */ + WCHAR *ptr, buffer[512]; + UINT msgGetTextLen = (msg == CB_GETLBTEXT) ? CB_GETLBTEXTLEN : LB_GETTEXTLEN; + LRESULT len = 0;
- ret = callback( hwnd, msg, wParam, (LPARAM)buffer, result, arg ); + callback( hwnd, msgGetTextLen, wParam, 0, &len, arg ); + if (!(ptr = get_buffer( buffer, sizeof(buffer), (len + 1) * sizeof(WCHAR) ))) break; + ret = callback( hwnd, msg, wParam, (LPARAM)ptr, result, arg ); if (*result >= 0) { DWORD len; RtlUnicodeToMultiByteN( (LPSTR)lParam, ~0u, &len, - buffer, (strlenW(buffer) + 1) * sizeof(WCHAR) ); + ptr, (strlenW(ptr) + 1) * sizeof(WCHAR) ); *result = len - 1; } + free_buffer( buffer, ptr ); } else ret = callback( hwnd, msg, wParam, lParam, result, arg ); break; @@ -777,15 +782,20 @@ static LRESULT WINPROC_CallProcWtoA( winproc_callback_t callback, HWND hwnd, UIN case CB_GETLBTEXT: if (lParam && WINPROC_TestLBForStr( hwnd, msg )) { - char buffer[512]; /* FIXME: fixed sized buffer */ + char *ptr, buffer[512]; + UINT msgGetTextLen = (msg == CB_GETLBTEXT) ? CB_GETLBTEXTLEN : LB_GETTEXTLEN; + LRESULT len = 0; + callback( hwnd, msgGetTextLen, wParam, 0, &len, arg ); + if (!(ptr = get_buffer( buffer, sizeof(buffer), len + 1 ))) break;
- ret = callback( hwnd, msg, wParam, (LPARAM)buffer, result, arg ); + ret = callback( hwnd, msg, wParam, (LPARAM)ptr, result, arg ); if (*result >= 0) { DWORD len; - RtlMultiByteToUnicodeN( (LPWSTR)lParam, ~0u, &len, buffer, strlen(buffer) + 1 ); + RtlMultiByteToUnicodeN( (LPWSTR)lParam, ~0u, &len, ptr, strlen(ptr) + 1 ); *result = len / sizeof(WCHAR) - 1; } + free_buffer( buffer, ptr ); } else ret = callback( hwnd, msg, wParam, lParam, result, arg ); break;
On 3/25/20 3:33 PM, Roman Pišl wrote:
Hello, this one fell off the list. Is it completely wrong or just nobody is interested? Or writing a test would help (which is easy in this case)? Also simple controls demo applications are affected - pasting large text into the control is enough for crash. Main question I guess is, does it really call with TEXTLEN messages on Windows?
I see, I was also thinking about that. Probably not. But is there a better solution? And does it harm? This is the same solution as for example handling WM_GETTEXTLENGTH. Ok, the patch is linked from the bug and stay in my working copy. And I will eventually make it better if I get some hints. Thanks. Dne 25. 03. 20 v 13:36 Nikolay Sivov napsal(a):
On 3/25/20 3:33 PM, Roman Pišl wrote:
Hello, this one fell off the list. Is it completely wrong or just nobody is interested? Or writing a test would help (which is easy in this case)? Also simple controls demo applications are affected - pasting large text into the control is enough for crash. Main question I guess is, does it really call with TEXTLEN messages on Windows?
participants (2)
-
Nikolay Sivov -
Roman Pišl