[PATCH 0/1] MR4682: user32: At least fail instantly on buffer overflow in winproc.

[Roman Pišl]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48559

Signed-off-by: Roman Pišl <rpisl(a)seznam.cz>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4682

[PATCH 1/1] user32: At least fail instantly on buffer overflow in winproc.

[Roman Pišl]

From: Roman Pišl <rpisl(a)seznam.cz>

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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/dlls/user32/winproc.c b/dlls/user32/winproc.c
index d2e4b29fc44..eff24bb3f2d 100644
--- a/dlls/user32/winproc.c
+++ b/dlls/user32/winproc.c
@@ -24,6 +24,7 @@
 #include "dbt.h"
 #include "wine/asm.h"
 #include "wine/debug.h"
+#include <assert.h>
 
 WINE_DEFAULT_DEBUG_CHANNEL(msg);
 WINE_DECLARE_DEBUG_CHANNEL(relay);
@@ -332,7 +333,10 @@ LRESULT WINPROC_CallProcAtoW( winproc_callback_t callback, HWND hwnd, UINT msg,
         {
             WCHAR buffer[512];  /* FIXME: fixed sized buffer */
 
+            buffer[ARRAY_SIZE(buffer) - 1] = 0;
             ret = callback( hwnd, msg, wParam, (LPARAM)buffer, result, arg );
+            assert(!buffer[ARRAY_SIZE(buffer) - 1]);
+
             if (*result >= 0)
             {
                 DWORD len;
@@ -604,7 +608,10 @@ static LRESULT WINPROC_CallProcWtoA( winproc_callback_t callback, HWND hwnd, UIN
         {
             char buffer[512];  /* FIXME: fixed sized buffer */
 
+            buffer[ARRAY_SIZE(buffer) - 1] = 0;
             ret = callback( hwnd, msg, wParam, (LPARAM)buffer, result, arg );
+            assert(!buffer[ARRAY_SIZE(buffer) - 1]);
+
             if (*result >= 0)
             {
                 DWORD len;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4682

[Jacek Caban (＠jacek)]

Is the problematic dispatch initiated by win32u or is it client-side dispatch? I guess that in case of win32u-initiated dispatches, we should perform A<->W conversion in win32u in more cases and then !4621 could cover it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4682#note_55886

[Roman Pišl]

On Thu Dec 14 09:53:17 2023 +0000, Jacek Caban wrote:
> Is the problematic dispatch initiated by win32u or is it client-side
> dispatch? I guess that in case of win32u-initiated dispatches, we should
> perform A<->W conversion in win32u in more cases and then !4621 could
> cover it.
> If no trace from win32u with WINEDEBUG=+msg means it is a client-side dispatch, then yes for the app in the bug 48559. Can't say anything about the bug 55960. I'm just proposing this patch into stable to save developer's time while investigating similar crashes.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4682#note_55970