[ros-diffs] [reactos] 01/01: [NTUser] Fix Strings and Format to Hooks

[James Tabor]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=568b6d0558d047df1826d…

commit 568b6d0558d047df1826d7438448432643d7fe5c
Author:     James Tabor <james.tabor(a)reactos.org>
AuthorDate: Fri Apr 3 03:58:29 2020 -0500
Commit:     James Tabor <james.tabor(a)reactos.org>
CommitDate: Fri Apr 3 03:58:29 2020 -0500

    [NTUser] Fix Strings and Format to Hooks
    
    Allocate heap instead of data segment to be used for callbacks on user side.
    Move and correct initial hook call out setup. Use it in more than one hook call.
    
    This fixes issues with strings out of alignment and use of kernel pointers.
    
    See CORE-13907 and CORE-16769. KsStudio still needs retested.
    
    Small wow update.
---
 win32ss/user/ntuser/callback.c     |  23 ++++--
 win32ss/user/ntuser/callback.h     |   1 +
 win32ss/user/ntuser/message.c      | 153 ++++++++++++++++++++++++++++++++++++-
 win32ss/user/ntuser/window.c       |  83 ++------------------
 win32ss/user/user32/windows/hook.c |  32 +++++---
 5 files changed, 195 insertions(+), 97 deletions(-)

diff --git a/win32ss/user/ntuser/callback.c b/win32ss/user/ntuser/callback.c
index 1497d72a892..7bcc65f5962 100644
--- a/win32ss/user/ntuser/callback.c
+++ b/win32ss/user/ntuser/callback.c
@@ -610,7 +610,12 @@ co_IntCallHookProc(INT HookId,
      {
          pCWP = (CWPSTRUCT*) lParam;
          ArgumentLength = sizeof(CWP_Struct);
-         lParamSize = lParamMemorySize(pCWP->message, pCWP->wParam,
pCWP->lParam);
+         if ( pCWP->message == WM_CREATE || pCWP->message == WM_NCCREATE )
+         {
+             lParamSize = sizeof(CREATESTRUCTW);
+         }
+         else
+             lParamSize = lParamMemorySize(pCWP->message, pCWP->wParam,
pCWP->lParam);
          ArgumentLength += lParamSize;
          break;
       }
@@ -618,7 +623,12 @@ co_IntCallHookProc(INT HookId,
       {
          pCWPR = (CWPRETSTRUCT*) lParam;
          ArgumentLength = sizeof(CWPR_Struct);
-         lParamSize = lParamMemorySize(pCWPR->message, pCWPR->wParam,
pCWPR->lParam);
+         if ( pCWPR->message == WM_CREATE || pCWPR->message == WM_NCCREATE )
+         {
+             lParamSize = sizeof(CREATESTRUCTW);
+         }
+         else
+             lParamSize = lParamMemorySize(pCWPR->message, pCWPR->wParam,
pCWPR->lParam);
          ArgumentLength += lParamSize;
          break;
       }
@@ -639,7 +649,7 @@ co_IntCallHookProc(INT HookId,
    Argument = IntCbAllocateMemory(ArgumentLength);
    if (NULL == Argument)
    {
-      ERR("HookProc callback failed: out of memory\n");
+      ERR("HookProc callback %d failed: out of memory
%d\n",HookId,ArgumentLength);
       goto Fault_Exit;
    }
    Common = (PHOOKPROC_CALLBACK_ARGUMENTS) Argument;
@@ -671,9 +681,10 @@ co_IntCallHookProc(INT HookId,
                CbtCreatewndExtra = (PHOOKPROC_CBT_CREATEWND_EXTRA_ARGUMENTS) Extra;
                RtlCopyMemory( &CbtCreatewndExtra->Cs, CbtCreateWnd->lpcs,
sizeof(CREATESTRUCTW) );
                CbtCreatewndExtra->WndInsertAfter = CbtCreateWnd->hwndInsertAfter;
-               CbtCreatewndExtra->Cs.lpszClass = CbtCreateWnd->lpcs->lpszClass;
-               CbtCreatewndExtra->Cs.lpszName = CbtCreateWnd->lpcs->lpszName;
+               CbtCreatewndExtra->Cs.lpszClass   =
CbtCreateWnd->lpcs->lpszClass;
+               CbtCreatewndExtra->Cs.lpszName    =
CbtCreateWnd->lpcs->lpszName;
                Common->lParam = (LPARAM) (Extra - (PCHAR) Common);
+               //ERR("HCBT_CREATEWND: hWnd %p Csw %p Name %p Class %p\n",
Common->wParam, CbtCreateWnd->lpcs, CbtCreateWnd->lpcs->lpszName,
CbtCreateWnd->lpcs->lpszClass);
                break;
             case HCBT_CLICKSKIPPED:
                RtlCopyMemory(Extra, (PVOID) lParam, sizeof(MOUSEHOOKSTRUCT));
@@ -754,7 +765,7 @@ co_IntCallHookProc(INT HookId,
    {
       if ( iTheId != HookId ) // Hook ID can change.
       {
-          ERR("Failure to make Callback %d! Status 0x%x\n",HookId,Status);
+          ERR("Failure to make Callback %d! Status 0x%x ArgumentLength
%d\n",HookId,Status,ArgumentLength);
           iTheId = HookId;
       }
       goto Fault_Exit;
diff --git a/win32ss/user/ntuser/callback.h b/win32ss/user/ntuser/callback.h
index 2eee4e9b4dd..c83be89a976 100644
--- a/win32ss/user/ntuser/callback.h
+++ b/win32ss/user/ntuser/callback.h
@@ -75,3 +75,4 @@ HANDLE FASTCALL co_IntCopyImage(HANDLE,UINT,INT,INT,UINT);
 BOOL FASTCALL co_IntSetWndIcons(VOID);
 VOID FASTCALL co_IntDeliverUserAPC(VOID);
 VOID FASTCALL co_IntSetupOBM(VOID);
+BOOL FASTCALL IntMsgCreateStructW(PWND,CREATESTRUCTW*,CREATESTRUCTW*,PVOID*,PVOID*);
diff --git a/win32ss/user/ntuser/message.c b/win32ss/user/ntuser/message.c
index aad83242943..5c6a3fc66a4 100644
--- a/win32ss/user/ntuser/message.c
+++ b/win32ss/user/ntuser/message.c
@@ -577,37 +577,184 @@ GetWakeMask(UINT first, UINT last )
     return mask;
 }
 
+//
+// Pass Strings to User Heap Space for Message Hook Callbacks.
+//
+BOOL
+FASTCALL
+IntMsgCreateStructW(
+    PWND Window,
+    CREATESTRUCTW *pCsw,
+    CREATESTRUCTW *Cs,
+    PVOID *ppszClass,
+    PVOID *ppszName )
+{
+    PLARGE_STRING WindowName;
+    PUNICODE_STRING ClassName;
+    PVOID pszClass = NULL, pszName = NULL;
+
+    /* Fill the new CREATESTRUCTW */
+    RtlCopyMemory(pCsw, Cs, sizeof(CREATESTRUCTW));
+    pCsw->style = Window->style; /* HCBT_CREATEWND needs the real window style */
+
+    WindowName = (PLARGE_STRING)   Cs->lpszName;
+    ClassName  = (PUNICODE_STRING) Cs->lpszClass;
+
+    // Based on the assumption this is from "unicode source" user32, ReactOS,
answer is yes.
+    if (!IS_ATOM(ClassName->Buffer))
+    {
+        if (ClassName->Length)
+        {
+            if (Window->state & WNDS_ANSICREATOR)
+            {
+                ANSI_STRING AnsiString;
+                AnsiString.MaximumLength =
(USHORT)RtlUnicodeStringToAnsiSize(ClassName)+sizeof(CHAR);
+                pszClass = UserHeapAlloc(AnsiString.MaximumLength);
+                if (!pszClass)
+                {
+                    ERR("UserHeapAlloc() failed!\n");
+                    return FALSE;
+                }
+                RtlZeroMemory(pszClass, AnsiString.MaximumLength);
+                AnsiString.Buffer = (PCHAR)pszClass;
+                RtlUnicodeStringToAnsiString(&AnsiString, ClassName, FALSE);
+            }
+            else
+            {
+                UNICODE_STRING UnicodeString;
+                UnicodeString.MaximumLength = ClassName->Length +
sizeof(UNICODE_NULL);
+                pszClass = UserHeapAlloc(UnicodeString.MaximumLength);
+                if (!pszClass)
+                {
+                    ERR("UserHeapAlloc() failed!\n");
+                    return FALSE;
+                }
+                RtlZeroMemory(pszClass, UnicodeString.MaximumLength);
+                UnicodeString.Buffer = (PWSTR)pszClass;
+                RtlCopyUnicodeString(&UnicodeString, ClassName);
+            }
+            *ppszClass = pszClass;
+            pCsw->lpszClass = UserHeapAddressToUser(pszClass);
+        }
+        else
+        {
+            pCsw->lpszClass = NULL;
+        }
+    }
+    else
+    {
+        pCsw->lpszClass = ClassName->Buffer;
+    }
+    if (WindowName->Length)
+    {
+        UNICODE_STRING Name;
+        Name.Buffer = WindowName->Buffer;
+        Name.Length = (USHORT)min(WindowName->Length, MAXUSHORT); // FIXME:
LARGE_STRING truncated
+        Name.MaximumLength = (USHORT)min(WindowName->MaximumLength, MAXUSHORT);
+
+        if (Window->state & WNDS_ANSICREATOR)
+        {
+            ANSI_STRING AnsiString;
+            AnsiString.MaximumLength = (USHORT)RtlUnicodeStringToAnsiSize(&Name) +
sizeof(CHAR);
+            pszName = UserHeapAlloc(AnsiString.MaximumLength);
+            if (!pszName)
+            {
+               ERR("UserHeapAlloc() failed!\n");
+               return FALSE;
+            }
+            RtlZeroMemory(pszName, AnsiString.MaximumLength);
+            AnsiString.Buffer = (PCHAR)pszName;
+            RtlUnicodeStringToAnsiString(&AnsiString, &Name, FALSE);
+        }
+        else
+        {
+            UNICODE_STRING UnicodeString;
+            UnicodeString.MaximumLength = Name.Length + sizeof(UNICODE_NULL);
+            pszName = UserHeapAlloc(UnicodeString.MaximumLength);
+            if (!pszName)
+            {
+               ERR("UserHeapAlloc() failed!\n");
+               return FALSE;
+            }
+            RtlZeroMemory(pszName, UnicodeString.MaximumLength);
+            UnicodeString.Buffer = (PWSTR)pszName;
+            RtlCopyUnicodeString(&UnicodeString, &Name);
+        }
+        *ppszName = pszName;
+        pCsw->lpszName = UserHeapAddressToUser(pszName);
+    }
+    else
+    {
+        pCsw->lpszName = NULL;
+    }
+
+    return TRUE;
+}
+
 static VOID FASTCALL
-IntCallWndProc( PWND Window, HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam)
+IntCallWndProc( PWND Window, HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam )
 {
     BOOL SameThread = FALSE;
     CWPSTRUCT CWP;
+    PVOID pszClass = NULL, pszName = NULL;
+    CREATESTRUCTW Csw;
+
+    //// Check for a hook to eliminate overhead. ////
+    if ( !ISITHOOKED(WH_CALLWNDPROC) &&
!(Window->head.rpdesk->pDeskInfo->fsHooks & HOOKID_TO_FLAG(WH_CALLWNDPROC))
)
+        return;
 
     if (Window->head.pti == ((PTHREADINFO)PsGetCurrentThreadWin32Thread()))
         SameThread = TRUE;
 
+    if ( Msg == WM_CREATE || Msg == WM_NCCREATE )
+    {   //
+        // String pointers are in user heap space, like WH_CBT HCBT_CREATEWND.
+        //
+        if (!IntMsgCreateStructW( Window, &Csw, (CREATESTRUCTW *)lParam,
&pszClass, &pszName ))
+            return;
+        lParam = (LPARAM)&Csw;
+    }
+
     CWP.hwnd    = hWnd;
     CWP.message = Msg;
     CWP.wParam  = wParam;
     CWP.lParam  = lParam;
     co_HOOK_CallHooks( WH_CALLWNDPROC, HC_ACTION, SameThread, (LPARAM)&CWP );
+
+    if (pszName)  UserHeapFree(pszName);
+    if (pszClass) UserHeapFree(pszClass);
 }
 
 static VOID FASTCALL
-IntCallWndProcRet ( PWND Window, HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam,
LRESULT *uResult)
+IntCallWndProcRet( PWND Window, HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam,
LRESULT *uResult )
 {
     BOOL SameThread = FALSE;
     CWPRETSTRUCT CWPR;
+    PVOID pszClass = NULL, pszName = NULL;
+    CREATESTRUCTW Csw;
+
+    if ( !ISITHOOKED(WH_CALLWNDPROCRET) &&
!(Window->head.rpdesk->pDeskInfo->fsHooks &
HOOKID_TO_FLAG(WH_CALLWNDPROCRET)) )
+        return;
 
     if (Window->head.pti == ((PTHREADINFO)PsGetCurrentThreadWin32Thread()))
         SameThread = TRUE;
 
+    if ( Msg == WM_CREATE || Msg == WM_NCCREATE )
+    {
+        if (!IntMsgCreateStructW( Window, &Csw, (CREATESTRUCTW *)lParam,
&pszClass, &pszName ))
+            return;
+        lParam = (LPARAM)&Csw;
+    }
+
     CWPR.hwnd    = hWnd;
     CWPR.message = Msg;
     CWPR.wParam  = wParam;
     CWPR.lParam  = lParam;
     CWPR.lResult = uResult ? (*uResult) : 0;
     co_HOOK_CallHooks( WH_CALLWNDPROCRET, HC_ACTION, SameThread, (LPARAM)&CWPR );
+
+    if (pszName)  UserHeapFree(pszName);
+    if (pszClass) UserHeapFree(pszClass);
 }
 
 static LRESULT handle_internal_message( PWND pWnd, UINT msg, WPARAM wparam, LPARAM lparam
)
@@ -2893,7 +3040,7 @@ DWORD
 APIENTRY
 NtUserWaitForInputIdle( IN HANDLE hProcess,
                         IN DWORD dwMilliseconds,
-                        IN BOOL Unknown2)
+                        IN BOOL bSharedWow)
 {
     PEPROCESS Process;
     PPROCESSINFO W32Process;
diff --git a/win32ss/user/ntuser/window.c b/win32ss/user/ntuser/window.c
index 9994a6b23ea..c70b6081191 100644
--- a/win32ss/user/ntuser/window.c
+++ b/win32ss/user/ntuser/window.c
@@ -2102,6 +2102,10 @@ co_UserCreateWindowEx(CREATESTRUCTW* Cs,
    UserDereferenceObject(Window);
    ObDereferenceObject(WinSta);
 
+   /* NCCREATE, WM_NCCALCSIZE and Hooks need the original values */
+   Cs->lpszName = (LPCWSTR) WindowName;
+   Cs->lpszClass = (LPCWSTR) ClassName;
+
    //// Check for a hook to eliminate overhead. ////
    if ( ISITHOOKED(WH_CBT) ||  (pti->rpdesk->pDeskInfo->fsHooks &
HOOKID_TO_FLAG(WH_CBT)) )
    {
@@ -2114,79 +2118,10 @@ co_UserCreateWindowEx(CREATESTRUCTW* Cs,
       	 goto cleanup;
       }
 
-      /* Fill the new CREATESTRUCTW */
-      RtlCopyMemory(pCsw, Cs, sizeof(CREATESTRUCTW));
-      pCsw->style = Window->style; /* HCBT_CREATEWND needs the real window style
*/
-
-      // Based on the assumption this is from "unicode source" user32, ReactOS,
answer is yes.
-      if (!IS_ATOM(ClassName->Buffer))
-      {
-         if (Window->state & WNDS_ANSICREATOR)
-         {
-            ANSI_STRING AnsiString;
-            AnsiString.MaximumLength =
(USHORT)RtlUnicodeStringToAnsiSize(ClassName)+sizeof(CHAR);
-            pszClass = UserHeapAlloc(AnsiString.MaximumLength);
-            if (!pszClass)
-            {
-               ERR("UserHeapAlloc() failed!\n");
-               goto cleanup;
-            }
-            RtlZeroMemory(pszClass, AnsiString.MaximumLength);
-            AnsiString.Buffer = (PCHAR)pszClass;
-            RtlUnicodeStringToAnsiString(&AnsiString, ClassName, FALSE);
-         }
-         else
-         {
-            UNICODE_STRING UnicodeString;
-            UnicodeString.MaximumLength = ClassName->Length + sizeof(UNICODE_NULL);
-            pszClass = UserHeapAlloc(UnicodeString.MaximumLength);
-            if (!pszClass)
-            {
-               ERR("UserHeapAlloc() failed!\n");
-               goto cleanup;
-            }
-            RtlZeroMemory(pszClass, UnicodeString.MaximumLength);
-            UnicodeString.Buffer = (PWSTR)pszClass;
-            RtlCopyUnicodeString(&UnicodeString, ClassName);
-         }
-         pCsw->lpszClass = UserHeapAddressToUser(pszClass);
-      }
-      if (WindowName->Length)
+      if (!IntMsgCreateStructW( Window, pCsw, Cs, &pszClass, &pszName ) )
       {
-         UNICODE_STRING Name;
-         Name.Buffer = WindowName->Buffer;
-         Name.Length = (USHORT)min(WindowName->Length, MAXUSHORT); // FIXME:
LARGE_STRING truncated
-         Name.MaximumLength = (USHORT)min(WindowName->MaximumLength, MAXUSHORT);
-
-         if (Window->state & WNDS_ANSICREATOR)
-         {
-            ANSI_STRING AnsiString;
-            AnsiString.MaximumLength = (USHORT)RtlUnicodeStringToAnsiSize(&Name) +
sizeof(CHAR);
-            pszName = UserHeapAlloc(AnsiString.MaximumLength);
-            if (!pszName)
-            {
-               ERR("UserHeapAlloc() failed!\n");
-               goto cleanup;
-            }
-            RtlZeroMemory(pszName, AnsiString.MaximumLength);
-            AnsiString.Buffer = (PCHAR)pszName;
-            RtlUnicodeStringToAnsiString(&AnsiString, &Name, FALSE);
-         }
-         else
-         {
-            UNICODE_STRING UnicodeString;
-            UnicodeString.MaximumLength = Name.Length + sizeof(UNICODE_NULL);
-            pszName = UserHeapAlloc(UnicodeString.MaximumLength);
-            if (!pszName)
-            {
-               ERR("UserHeapAlloc() failed!\n");
-               goto cleanup;
-            }
-            RtlZeroMemory(pszName, UnicodeString.MaximumLength);
-            UnicodeString.Buffer = (PWSTR)pszName;
-            RtlCopyUnicodeString(&UnicodeString, &Name);
-         }
-         pCsw->lpszName = UserHeapAddressToUser(pszName);
+          ERR("IntMsgCreateStructW() failed!\n");
+          goto cleanup;
       }
 
       pCbtCreate->lpcs = pCsw;
@@ -2207,10 +2142,6 @@ co_UserCreateWindowEx(CREATESTRUCTW* Cs,
       hwndInsertAfter = pCbtCreate->hwndInsertAfter;
    }
 
-   /* NCCREATE and WM_NCCALCSIZE need the original values */
-   Cs->lpszName = (LPCWSTR) WindowName;
-   Cs->lpszClass = (LPCWSTR) ClassName;
-
    if ((Cs->style & (WS_CHILD|WS_POPUP)) == WS_CHILD)
    {
       if (ParentWindow != co_GetDesktopWindow(Window))
diff --git a/win32ss/user/user32/windows/hook.c b/win32ss/user/user32/windows/hook.c
index f525bf1cc5a..78b1a37acb3 100644
--- a/win32ss/user/user32/windows/hook.c
+++ b/win32ss/user/user32/windows/hook.c
@@ -554,8 +554,8 @@ NTSTATUS WINAPI
 User32CallHookProcFromKernel(PVOID Arguments, ULONG ArgumentLength)
 {
   PHOOKPROC_CALLBACK_ARGUMENTS Common;
-  CREATESTRUCTW Csw;
-  CBT_CREATEWNDW CbtCreatewndw;
+  CREATESTRUCTW *pCsw = NULL;
+  CBT_CREATEWNDW *pCbtCreatewndw = NULL;
   PHOOKPROC_CBT_CREATEWND_EXTRA_ARGUMENTS CbtCreatewndExtra = NULL;
   KBDLLHOOKSTRUCT KeyboardLlData, *pKeyboardLlData;
   MSLLHOOKSTRUCT MouseLlData, *pMouseLlData;
@@ -608,12 +608,18 @@ User32CallHookProcFromKernel(PVOID Arguments, ULONG ArgumentLength)
         case HCBT_CREATEWND:
           CbtCreatewndExtra = (PHOOKPROC_CBT_CREATEWND_EXTRA_ARGUMENTS)
                               ((PCHAR) Common + Common->lParam);
-          RtlCopyMemory(&Csw, &CbtCreatewndExtra->Cs, sizeof(CREATESTRUCTW));
-          CbtCreatewndw.lpcs = &Csw;
-          CbtCreatewndw.hwndInsertAfter = CbtCreatewndExtra->WndInsertAfter;
+
+          pCbtCreatewndw = (CBT_CREATEWNDW*)HeapAlloc(GetProcessHeap(), 0,
sizeof(CBT_CREATEWNDW));
+          RtlCopyMemory(pCbtCreatewndw, CbtCreatewndExtra, sizeof(CBT_CREATEWNDW));
+
+          pCsw = (CREATESTRUCTW*)HeapAlloc(GetProcessHeap(), 0, sizeof(CREATESTRUCTW));
+          RtlCopyMemory(pCsw, &CbtCreatewndExtra->Cs, sizeof(CREATESTRUCTW));
+
+          pCbtCreatewndw->lpcs = pCsw;
+          pCbtCreatewndw->hwndInsertAfter = CbtCreatewndExtra->WndInsertAfter;
           wParam = Common->wParam;
-          lParam = (LPARAM) &CbtCreatewndw;
-          //ERR("HCBT_CREATEWND: hWnd 0x%x Name 0x%x Class 0x%x\n",
Common->wParam, Csw.lpszName, Csw.lpszClass);
+          lParam = (LPARAM) pCbtCreatewndw;
+          //ERR("HCBT_CREATEWND: hWnd %p Csw %p Name %p Class %p\n",
Common->wParam, pCsw, pCsw->lpszName, pCsw->lpszClass);
           break;
         case HCBT_CLICKSKIPPED:
             pMHook = (PMOUSEHOOKSTRUCT)((PCHAR) Common + Common->lParam);
@@ -665,11 +671,13 @@ User32CallHookProcFromKernel(PVOID Arguments, ULONG ArgumentLength)
       switch(Common->Code)
       {
         case HCBT_CREATEWND:
-          CbtCreatewndExtra->WndInsertAfter = CbtCreatewndw.hwndInsertAfter;
-          CbtCreatewndExtra->Cs.x = CbtCreatewndw.lpcs->x;
-          CbtCreatewndExtra->Cs.y = CbtCreatewndw.lpcs->y;
-          CbtCreatewndExtra->Cs.cx = CbtCreatewndw.lpcs->cx;
-          CbtCreatewndExtra->Cs.cy = CbtCreatewndw.lpcs->cy;
+          CbtCreatewndExtra->WndInsertAfter = pCbtCreatewndw->hwndInsertAfter;
+          CbtCreatewndExtra->Cs.x  = pCbtCreatewndw->lpcs->x;
+          CbtCreatewndExtra->Cs.y  = pCbtCreatewndw->lpcs->y;
+          CbtCreatewndExtra->Cs.cx = pCbtCreatewndw->lpcs->cx;
+          CbtCreatewndExtra->Cs.cy = pCbtCreatewndw->lpcs->cy;
+          HeapFree(GetProcessHeap(), 0, pCsw);
+          HeapFree(GetProcessHeap(), 0, pCbtCreatewndw);
           break;
       }
       break;