[v2 PATCH] user32/defwnd: Partially protect WM_SETTEXT handlers from invalid input

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov <nsivov at codeweavers.com>
---

v2: check moved out of default procedure, removed duplicated checks

 dlls/user32/defwnd.c    | 40 ++++++++++++++++++++++++++--------------
 dlls/user32/tests/msg.c | 18 ++++++++++++++++++
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/dlls/user32/defwnd.c b/dlls/user32/defwnd.c
index 6fbaf1ef18..235c139baf 100644
--- a/dlls/user32/defwnd.c
+++ b/dlls/user32/defwnd.c
@@ -83,16 +83,21 @@ static void DEFWND_HandleWindowPosChanged( HWND hwnd, const WINDOWPOS *winpos )
  *
  * Set the window text.
  */
-static void DEFWND_SetTextA( HWND hwnd, LPCSTR text )
+static LRESULT DEFWND_SetTextA( HWND hwnd, LPCSTR text )
 {
     int count;
     WCHAR *textW;
     WND *wndPtr;
 
+    /* check for string, as static icons, bitmaps (SS_ICON, SS_BITMAP)
+     * may have child window IDs instead of window name */
+    if (text && IS_INTRESOURCE(text))
+        return 0;
+
     if (!text) text = "";
     count = MultiByteToWideChar( CP_ACP, 0, text, -1, NULL, 0 );
 
-    if (!(wndPtr = WIN_GetPtr( hwnd ))) return;
+    if (!(wndPtr = WIN_GetPtr( hwnd ))) return 0;
     if ((textW = HeapAlloc(GetProcessHeap(), 0, count * sizeof(WCHAR))))
     {
         HeapFree(GetProcessHeap(), 0, wndPtr->text);
@@ -111,6 +116,8 @@ static void DEFWND_SetTextA( HWND hwnd, LPCSTR text )
     WIN_ReleasePtr( wndPtr );
 
     USER_Driver->pSetWindowText( hwnd, textW );
+
+    return 1;
 }
 
 /***********************************************************************
@@ -118,16 +125,21 @@ static void DEFWND_SetTextA( HWND hwnd, LPCSTR text )
  *
  * Set the window text.
  */
-static void DEFWND_SetTextW( HWND hwnd, LPCWSTR text )
+static LRESULT DEFWND_SetTextW( HWND hwnd, LPCWSTR text )
 {
     static const WCHAR empty_string[] = {0};
     WND *wndPtr;
     int count;
 
+    /* check for string, as static icons, bitmaps (SS_ICON, SS_BITMAP)
+     * may have child window IDs instead of window name */
+    if (text && IS_INTRESOURCE(text))
+        return 0;
+
     if (!text) text = empty_string;
     count = strlenW(text) + 1;
 
-    if (!(wndPtr = WIN_GetPtr( hwnd ))) return;
+    if (!(wndPtr = WIN_GetPtr( hwnd ))) return 0;
     HeapFree(GetProcessHeap(), 0, wndPtr->text);
     if ((wndPtr->text = HeapAlloc(GetProcessHeap(), 0, count * sizeof(WCHAR))))
     {
@@ -146,6 +158,8 @@ static void DEFWND_SetTextW( HWND hwnd, LPCWSTR text )
     WIN_ReleasePtr( wndPtr );
 
     USER_Driver->pSetWindowText( hwnd, text );
+
+    return 1;
 }
 
 /***********************************************************************
@@ -783,10 +797,8 @@ LRESULT WINAPI DefWindowProcA( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam
         if (lParam)
         {
             CREATESTRUCTA *cs = (CREATESTRUCTA *)lParam;
-            /* check for string, as static icons, bitmaps (SS_ICON, SS_BITMAP)
-             * may have child window IDs instead of window name */
-            if (!IS_INTRESOURCE(cs->lpszName))
-                DEFWND_SetTextA( hwnd, cs->lpszName );
+
+            DEFWND_SetTextA( hwnd, cs->lpszName );
             result = 1;
 
             if(cs->style & (WS_HSCROLL | WS_VSCROLL))
@@ -822,7 +834,8 @@ LRESULT WINAPI DefWindowProcA( HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam
         break;
 
     case WM_SETTEXT:
-        DEFWND_SetTextA( hwnd, (LPCSTR)lParam );
+        if (!DEFWND_SetTextA( hwnd, (LPCSTR)lParam ))
+            break;
         if( (GetWindowLongW( hwnd, GWL_STYLE ) & WS_CAPTION) == WS_CAPTION )
             NC_HandleNCPaint( hwnd , (HRGN)1 );  /* Repaint caption */
         result = 1; /* success. FIXME: check text length */
@@ -933,10 +946,8 @@ LRESULT WINAPI DefWindowProcW(
         if (lParam)
         {
             CREATESTRUCTW *cs = (CREATESTRUCTW *)lParam;
-            /* check for string, as static icons, bitmaps (SS_ICON, SS_BITMAP)
-             * may have child window IDs instead of window name */
-            if (!IS_INTRESOURCE(cs->lpszName))
-                DEFWND_SetTextW( hwnd, cs->lpszName );
+
+            DEFWND_SetTextW( hwnd, cs->lpszName );
             result = 1;
 
             if(cs->style & (WS_HSCROLL | WS_VSCROLL))
@@ -969,7 +980,8 @@ LRESULT WINAPI DefWindowProcW(
         break;
 
     case WM_SETTEXT:
-        DEFWND_SetTextW( hwnd, (LPCWSTR)lParam );
+        if (!DEFWND_SetTextW( hwnd, (LPCWSTR)lParam ))
+            break;
         if( (GetWindowLongW( hwnd, GWL_STYLE ) & WS_CAPTION) == WS_CAPTION )
             NC_HandleNCPaint( hwnd , (HRGN)1 );  /* Repaint caption */
         result = 1; /* success. FIXME: check text length */
diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c
index fbadf7d29a..760a9ea7a4 100644
--- a/dlls/user32/tests/msg.c
+++ b/dlls/user32/tests/msg.c
@@ -14438,6 +14438,7 @@ static void test_defwinproc(void)
     INT x, y;
     LRESULT res;
     struct rbuttonup_thread_data data;
+    char buffA[64];
     HANDLE thread;
 
     hwnd = CreateWindowExA(0, "TestWindowClass", "test_defwndproc",
@@ -14445,6 +14446,23 @@ static void test_defwinproc(void)
     assert(hwnd);
     flush_events();
 
+    buffA[0] = 0;
+    GetWindowTextA(hwnd, buffA, sizeof(buffA)/sizeof(*buffA));
+    ok(!strcmp(buffA, "test_defwndproc"), "unexpected window text, %s\n", buffA);
+
+    /* Zero high word of the lParam */
+    res = DefWindowProcA(hwnd, WM_SETTEXT, 0, 0x1234);
+    ok(res == 0, "WM_SETTEXT was expected to fail, %ld\n", res);
+
+    GetWindowTextA(hwnd, buffA, sizeof(buffA)/sizeof(*buffA));
+    ok(!strcmp(buffA, "test_defwndproc"), "unexpected window text, %s\n", buffA);
+
+    res = DefWindowProcW(hwnd, WM_SETTEXT, 0, 0x1234);
+    ok(res == 0, "WM_SETTEXT was expected to fail, %ld\n", res);
+
+    GetWindowTextA(hwnd, buffA, sizeof(buffA)/sizeof(*buffA));
+    ok(!strcmp(buffA, "test_defwndproc"), "unexpected window text, %s\n", buffA);
+
     GetCursorPos(&pos);
     GetWindowRect(hwnd, &rect);
     x = (rect.left+rect.right) / 2;
-- 
2.11.0