[ros-diffs] [reactos] 01/01: [NTUSER][USER32] Refactor NtUserLoadKeyboardLayoutEx (#5107)

[Katayama Hirofumi MZ]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=77e6348f5fce69070852d…

commit 77e6348f5fce69070852df0e516f649012930d30
Author:     Katayama Hirofumi MZ <katayama.hirofumi.mz(a)gmail.com>
AuthorDate: Sun Mar 5 11:41:32 2023 +0900
Commit:     GitHub <noreply(a)github.com>
CommitDate: Sun Mar 5 11:41:32 2023 +0900

    [NTUSER][USER32] Refactor NtUserLoadKeyboardLayoutEx (#5107)
    
    - Split some code of NtUserLoadKeyboardLayoutEx to newly-added
co_IntLoadKeyboardLayoutEx helper function.
    - Modify NtUserLoadKeyboardLayoutEx prototype.
    - Move co_UserImmLoadLayout code.
    - Implement KLF_REORDER.
    - Rename UserLoadKbdLayout as co_UserLoadKbdLayout.
    - Improve LoadKeyboardLayoutEx.
    CORE-11700
---
 win32ss/include/ntuser.h            |   8 +-
 win32ss/user/ntuser/kbdlayout.c     | 257 ++++++++++++++++++++++--------------
 win32ss/user/user32/windows/input.c |  10 +-
 3 files changed, 165 insertions(+), 110 deletions(-)

diff --git a/win32ss/include/ntuser.h b/win32ss/include/ntuser.h
index cd7fcdf7034..656d3b443fa 100644
--- a/win32ss/include/ntuser.h
+++ b/win32ss/include/ntuser.h
@@ -2736,12 +2736,12 @@ NtUserKillTimer(
 HKL
 NTAPI
 NtUserLoadKeyboardLayoutEx(
-    IN HANDLE Handle,
+    IN HANDLE hFile,
     IN DWORD offTable,
-    IN PUNICODE_STRING puszKeyboardName,
-    IN HKL hKL,
+    IN PVOID pTables,
+    IN HKL hOldKL,
     IN PUNICODE_STRING puszKLID,
-    IN DWORD dwKLID,
+    IN DWORD dwNewKL,
     IN UINT Flags);
 
 BOOL
diff --git a/win32ss/user/ntuser/kbdlayout.c b/win32ss/user/ntuser/kbdlayout.c
index 6ce1523d208..9a5c43d60a4 100644
--- a/win32ss/user/ntuser/kbdlayout.c
+++ b/win32ss/user/ntuser/kbdlayout.c
@@ -369,12 +369,12 @@ cleanup:
 }
 
 /*
- * UserLoadKbdLayout
+ * co_UserLoadKbdLayout
  *
  * Loads keyboard layout and creates KL object
  */
 static PKL
-UserLoadKbdLayout(PUNICODE_STRING pustrKLID, HKL hKL)
+co_UserLoadKbdLayout(PUNICODE_STRING pustrKLID, HKL hKL)
 {
     LCID lCid;
     CHARSETINFO cs;
@@ -846,6 +846,129 @@ IntUnloadKeyboardLayout(_Inout_ PWINSTATION_OBJECT pWinSta, _In_ HKL
hKL)
     return co_IntUnloadKeyboardLayoutEx(pWinSta, pKL, 0);
 }
 
+PIMEINFOEX FASTCALL co_UserImmLoadLayout(_In_ HKL hKL)
+{
+    PIMEINFOEX piiex;
+
+    if (!IS_IME_HKL(hKL) && !IS_CICERO_MODE())
+        return NULL;
+
+    piiex = ExAllocatePoolWithTag(PagedPool, sizeof(IMEINFOEX), USERTAG_IME);
+    if (!piiex)
+        return NULL;
+
+    if (!co_ClientImmLoadLayout(hKL, piiex))
+    {
+        ExFreePoolWithTag(piiex, USERTAG_IME);
+        return NULL;
+    }
+
+    return piiex;
+}
+
+HKL APIENTRY
+co_IntLoadKeyboardLayoutEx(
+    IN OUT PWINSTATION_OBJECT pWinSta,
+    IN HANDLE hSafeFile,
+    IN HKL hOldKL,
+    IN PUNICODE_STRING puszSafeKLID,
+    IN HKL hNewKL,
+    IN UINT Flags)
+{
+    PKL pOldKL, pNewKL;
+
+    UNREFERENCED_PARAMETER(hSafeFile);
+
+    if (hNewKL == NULL || (pWinSta->Flags & WSS_NOIO))
+        return NULL;
+
+    /* If hOldKL is specified, unload it and load new layput as default */
+    if (hOldKL && hOldKL != hNewKL)
+    {
+        pOldKL = UserHklToKbl(hOldKL);
+        if (pOldKL)
+            UserUnloadKbl(pOldKL);
+    }
+
+    /* FIXME: It seems KLF_RESET is only supported for WINLOGON */
+
+    /* Let's see if layout was already loaded. */
+    pNewKL = UserHklToKbl(hNewKL);
+    if (!pNewKL)
+    {
+        /* It wasn't, so load it. */
+        pNewKL = co_UserLoadKbdLayout(puszSafeKLID, hNewKL);
+        if (!pNewKL)
+            return NULL;
+
+        if (gspklBaseLayout)
+        {
+            /* Find last not unloaded layout */
+            PKL pLastKL = gspklBaseLayout->pklPrev;
+            while (pLastKL != gspklBaseLayout && (pLastKL->dwKL_Flags &
KLF_UNLOAD))
+                pLastKL = pLastKL->pklPrev;
+
+            /* Add new layout to the list */
+            pNewKL->pklNext = pLastKL->pklNext;
+            pNewKL->pklPrev = pLastKL;
+            pNewKL->pklNext->pklPrev = pNewKL;
+            pNewKL->pklPrev->pklNext = pNewKL;
+        }
+        else
+        {
+            /* This is the first layout */
+            pNewKL->pklNext = pNewKL;
+            pNewKL->pklPrev = pNewKL;
+            gspklBaseLayout = pNewKL;
+        }
+
+        pNewKL->piiex = co_UserImmLoadLayout(hNewKL);
+    }
+
+    /* If this layout was prepared to unload, undo it */
+    pNewKL->dwKL_Flags &= ~KLF_UNLOAD;
+
+    /* Reorder if necessary */
+    if (Flags & KLF_REORDER)
+        IntReorderKeyboardLayouts(pWinSta, pNewKL);
+
+    /* Activate this layout in current thread */
+    if (Flags & KLF_ACTIVATE)
+        co_UserActivateKbl(PsGetCurrentThreadWin32Thread(), pNewKL, Flags);
+
+    /* Send shell message */
+    if (!(Flags & KLF_NOTELLSHELL))
+        co_IntShellHookNotify(HSHELL_LANGUAGE, 0, (LPARAM)hNewKL);
+
+    /* FIXME: KLF_REPLACELANG */
+
+    return hNewKL;
+}
+
+HANDLE FASTCALL IntVerifyKeyboardFileHandle(HANDLE hFile)
+{
+    PFILE_OBJECT FileObject;
+    NTSTATUS Status;
+
+    if (hFile == INVALID_HANDLE_VALUE)
+        return NULL;
+
+    Status = ObReferenceObjectByHandle(hFile, FILE_READ_DATA, NULL, UserMode,
+                                       (PVOID*)&FileObject, NULL);
+    if (!NT_SUCCESS(Status))
+    {
+        ERR("0x%08X\n", Status);
+        return NULL;
+    }
+
+    /* FIXME: Is the file in the system directory? */
+
+    if (FileObject)
+        ObDereferenceObject(FileObject);
+
+    return hFile;
+}
+
 /* EXPORTS *******************************************************************/
 
 /*
@@ -1009,50 +1132,34 @@ cleanup:
     return bRet;
 }
 
-/* Win: xxxImmLoadLayout */
-PIMEINFOEX FASTCALL co_UserImmLoadLayout(_In_ HKL hKL)
-{
-    PIMEINFOEX piiex;
-
-    if (!IS_IME_HKL(hKL) && !IS_CICERO_MODE())
-        return NULL;
-
-    piiex = ExAllocatePoolWithTag(PagedPool, sizeof(IMEINFOEX), USERTAG_IME);
-    if (!piiex)
-        return NULL;
-
-    if (!co_ClientImmLoadLayout(hKL, piiex))
-    {
-        ExFreePoolWithTag(piiex, USERTAG_IME);
-        return NULL;
-    }
-
-    return piiex;
-}
-
 /*
  * NtUserLoadKeyboardLayoutEx
  *
  * Loads keyboard layout with given locale id
  *
- * NOTE: We adopt a different design from Microsoft's one for security reason.
- *       We don't use the 1st and 3rd parameters of NtUserLoadKeyboardLayoutEx.
+ * NOTE: We adopt a different design from Microsoft's one due to security reason.
+ *       We don't use the 3rd parameter of NtUserLoadKeyboardLayoutEx.
+ *       See https://bugtraq.securityfocus.com/detail/50056B96.6040306
  */
 HKL
-APIENTRY
+NTAPI
 NtUserLoadKeyboardLayoutEx(
-    IN HANDLE Handle, // hFile (See
downloads.securityfocus.com/vulnerabilities/exploits/43774.c)
-    IN DWORD offTable, // Offset to KbdTables
-    IN PUNICODE_STRING puszKeyboardName, // Not used?
-    IN HKL hklUnload,
-    IN PUNICODE_STRING pustrKLID,
-    IN DWORD hkl,
+    IN HANDLE hFile,
+    IN DWORD offTable,
+    IN PVOID pTables,
+    IN HKL hOldKL,
+    IN PUNICODE_STRING puszKLID,
+    IN DWORD dwNewKL,
     IN UINT Flags)
 {
-    HKL hklRet = NULL;
-    PKL pKl = NULL, pklLast;
+    HKL hRetKL;
     WCHAR Buffer[KL_NAMELENGTH];
-    UNICODE_STRING ustrSafeKLID;
+    UNICODE_STRING uszSafeKLID;
+    PWINSTATION_OBJECT pWinSta;
+    HANDLE hSafeFile;
+
+    UNREFERENCED_PARAMETER(offTable);
+    UNREFERENCED_PARAMETER(pTables);
 
     if (Flags & ~(KLF_ACTIVATE|KLF_NOTELLSHELL|KLF_REORDER|KLF_REPLACELANG|
                   KLF_SUBSTITUTE_OK|KLF_SETFORPROCESS|KLF_UNLOADPREVIOUS|
@@ -1063,14 +1170,12 @@ NtUserLoadKeyboardLayoutEx(
         return NULL;
     }
 
-    /* FIXME: It seems KLF_RESET is only supported for WINLOGON */
-
-    RtlInitEmptyUnicodeString(&ustrSafeKLID, Buffer, sizeof(Buffer));
+    RtlInitEmptyUnicodeString(&uszSafeKLID, Buffer, sizeof(Buffer));
     _SEH2_TRY
     {
-        ProbeForRead(pustrKLID, sizeof(*pustrKLID), 1);
-        ProbeForRead(pustrKLID->Buffer, sizeof(pustrKLID->Length), 1);
-        RtlCopyUnicodeString(&ustrSafeKLID, pustrKLID);
+        ProbeForRead(puszKLID, sizeof(*puszKLID), 1);
+        ProbeForRead(puszKLID->Buffer, sizeof(puszKLID->Length), 1);
+        RtlCopyUnicodeString(&uszSafeKLID, puszKLID);
     }
     _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
     {
@@ -1081,67 +1186,19 @@ NtUserLoadKeyboardLayoutEx(
 
     UserEnterExclusive();
 
-    /* If hklUnload is specified, unload it and load new layput as default */
-    if (hklUnload && (hklUnload != UlongToHandle(hkl)))
-    {
-        pKl = UserHklToKbl(hklUnload);
-        if (pKl)
-            UserUnloadKbl(pKl);
-    }
-
-    /* Let's see if layout was already loaded. */
-    pKl = UserHklToKbl(UlongToHandle(hkl));
-    if (!pKl)
-    {
-        /* It wasn't, so load it. */
-        pKl = UserLoadKbdLayout(&ustrSafeKLID, UlongToHandle(hkl));
-        if (!pKl)
-            goto cleanup;
-
-        if (gspklBaseLayout)
-        {
-            /* Find last not unloaded layout */
-            pklLast = gspklBaseLayout->pklPrev;
-            while (pklLast != gspklBaseLayout && pklLast->dwKL_Flags &
KLF_UNLOAD)
-                pklLast = pklLast->pklPrev;
-
-            /* Add new layout to the list */
-            pKl->pklNext = pklLast->pklNext;
-            pKl->pklPrev = pklLast;
-            pKl->pklNext->pklPrev = pKl;
-            pKl->pklPrev->pklNext = pKl;
-        }
-        else
-        {
-            /* This is the first layout */
-            pKl->pklNext = pKl;
-            pKl->pklPrev = pKl;
-            gspklBaseLayout = pKl;
-        }
-
-        pKl->piiex = co_UserImmLoadLayout(UlongToHandle(hkl));
-    }
-
-    /* If this layout was prepared to unload, undo it */
-    pKl->dwKL_Flags &= ~KLF_UNLOAD;
-
-    /* Activate this layout in current thread */
-    if (Flags & KLF_ACTIVATE)
-        co_UserActivateKbl(PsGetCurrentThreadWin32Thread(), pKl, Flags);
-
-    /* Send shell message */
-    if (!(Flags & KLF_NOTELLSHELL))
-        co_IntShellHookNotify(HSHELL_LANGUAGE, 0, (LPARAM)hkl);
-
-    /* Return hkl on success */
-    hklRet = UlongToHandle(hkl);
-
-    /* FIXME: KLF_REPLACELANG
-              KLF_REORDER */
+    hSafeFile = (hFile ? IntVerifyKeyboardFileHandle(hFile) : NULL);
+    pWinSta = IntGetProcessWindowStation(NULL);
+    hRetKL = co_IntLoadKeyboardLayoutEx(pWinSta,
+                                        hSafeFile,
+                                        hOldKL,
+                                        &uszSafeKLID,
+                                        (HKL)(DWORD_PTR)dwNewKL,
+                                        Flags);
+    if (hSafeFile)
+        ZwClose(hSafeFile);
 
-cleanup:
     UserLeave();
-    return hklRet;
+    return hRetKL;
 }
 
 /*
diff --git a/win32ss/user/user32/windows/input.c b/win32ss/user/user32/windows/input.c
index d3b7e114ba9..48b18b069da 100644
--- a/win32ss/user/user32/windows/input.c
+++ b/win32ss/user/user32/windows/input.c
@@ -721,9 +721,9 @@ VOID GetSystemLibraryPath(LPWSTR pszPath, INT cchPath, LPCWSTR
pszFileName)
 /*
  * @unimplemented
  *
- * NOTE: We adopt a different design from Microsoft's one for security reason.
+ * NOTE: We adopt a different design from Microsoft's one due to security reason.
+ *       See NtUserLoadKeyboardLayoutEx.
  */
-/* Win: LoadKeyboardLayoutWorker */
 HKL APIENTRY
 IntLoadKeyboardLayout(
     _In_    HKL     hklUnload,
@@ -733,7 +733,6 @@ IntLoadKeyboardLayout(
     _In_    BOOL    unknown5)
 {
     DWORD dwKLID, dwHKL, dwType, dwSize;
-    UNICODE_STRING ustrKbdName;
     UNICODE_STRING ustrKLID;
     WCHAR wszRegKey[256] = L"SYSTEM\\CurrentControlSet\\Control\\Keyboard
Layouts\\";
     WCHAR wszLayoutId[10], wszNewKLID[KL_NAMELENGTH], szImeFileName[80];
@@ -807,7 +806,7 @@ IntLoadKeyboardLayout(
                 szImeFileName[_countof(szImeFileName) - 1] = UNICODE_NULL;
                 GetSystemLibraryPath(szPath, _countof(szPath), szImeFileName);
 
-                /* We don't allow the invalid "IME File" values for
security reason */
+                /* We don't allow the invalid "IME File" values due to
security reason */
                 if (dwType != REG_SZ || szImeFileName[0] == 0 ||
                     wcscspn(szImeFileName, L":\\/") != wcslen(szImeFileName)
||
                     GetFileAttributesW(szPath) == INVALID_FILE_ATTRIBUTES) /* Does not
exist? */
@@ -833,9 +832,8 @@ IntLoadKeyboardLayout(
 
     dwHKL = MAKELONG(wLow, wHigh);
 
-    ZeroMemory(&ustrKbdName, sizeof(ustrKbdName));
     RtlInitUnicodeString(&ustrKLID, pwszKLID);
-    hNewKL = NtUserLoadKeyboardLayoutEx(NULL, 0, &ustrKbdName, NULL, &ustrKLID,
dwHKL, Flags);
+    hNewKL = NtUserLoadKeyboardLayoutEx(NULL, 0, NULL, hklUnload, &ustrKLID, dwHKL,
Flags);
     CliImmInitializeHotKeys(SETIMEHOTKEY_ADD, hNewKL);
     return hNewKL;
 }