[CTFMON] Some fixes for event handles management (reactos#8392)

- Validate the opened `WinSta0_DesktopSwitch` handle;

- Don't pass the `ahEvents` array containing a NULL pointer to
  `MsgWaitForMultipleObjects()`, but provide the correct number
  of non-NULL handles, otherwise the function fails and the code
  ends up busy-looping.

- Validate the `MsgWaitForMultipleObjects()` result: bail out if
  `WAIT_FAILED` is returned.

- Validate the event index passed to `CRegWatcher::InitEvent()`
  and `CRegWatcher::OnEvent()`.

- Rename `WATCHENTRY_MAX` to `WI_REGEVTS_MAX` and add it in the
  `WATCH_INDEX` enumeration.

- Fix some code comments.

Author: HBelusca

base/ctf/ctfmon/CRegWatcher.cpp

@@ -10,10 +10,10 @@
 #include <ime/indicml.h>
 
 // The event handles to use in watching
-HANDLE CRegWatcher::s_ahWatchEvents[WATCHENTRY_MAX] = { NULL };
+HANDLE CRegWatcher::s_ahWatchEvents[WI_REGEVTS_MAX] = { NULL };
 
 // The registry entries to watch
-WATCHENTRY CRegWatcher::s_WatchEntries[WATCHENTRY_MAX] =
+WATCHENTRY CRegWatcher::s_WatchEntries[WI_REGEVTS_MAX] =
 {
     { HKEY_CURRENT_USER,  TEXT("Keyboard Layout\\Toggle")                           }, // WI_TOGGLE
     { HKEY_LOCAL_MACHINE, TEXT("SOFTWARE\\Microsoft\\CTF\\TIP")                     }, // WI_MACHINE_TIF
@@ -114,6 +114,9 @@ CRegWatcher::InitEvent(
     _In_ SIZE_T iEvent,
     _In_ BOOL bResetEvent)
 {
+    if (iEvent >= _countof(s_ahWatchEvents))
+        return FALSE;
+
     // Reset the signal status
     if (bResetEvent)
         ::ResetEvent(s_ahWatchEvents[iEvent]);
@@ -315,6 +318,9 @@ VOID
 CRegWatcher::OnEvent(
     _In_ SIZE_T iEvent)
 {
+    if (iEvent >= _countof(s_ahWatchEvents))
+        return;
+
     InitEvent(iEvent, TRUE);
 
     switch (iEvent)

base/ctf/ctfmon/CRegWatcher.h

@@ -14,12 +14,10 @@ struct WATCHENTRY
     HKEY hKey;
 };
 
-#define WATCHENTRY_MAX 12
-
 struct CRegWatcher
 {
-    static HANDLE s_ahWatchEvents[WATCHENTRY_MAX];
-    static WATCHENTRY s_WatchEntries[WATCHENTRY_MAX];
+    static HANDLE s_ahWatchEvents[WI_REGEVTS_MAX];
+    static WATCHENTRY s_WatchEntries[WI_REGEVTS_MAX];
     static UINT s_nSysColorTimerId, s_nKbdToggleTimerId, s_nRegImxTimerId;
 
     static BOOL Init();

base/ctf/ctfmon/ctfmon.cpp

@@ -189,9 +189,9 @@ InitApp(
     _In_ HINSTANCE hInstance,
     _In_ LPTSTR lpCmdLine)
 {
-    g_hInst     = hInstance;    // Save the instance handle
+    g_hInst = hInstance; // Save the instance handle
 
-    g_bOnWow64  = cicIsWow64();   // Is the current process on WoW64?
+    g_bOnWow64 = cicIsWow64(); // Is the current process on WoW64?
     cicGetOSInfo(&g_uACP, &g_dwOsInfo); // Get OS info
 
     // Create a mutex for Cicero
@@ -227,7 +227,7 @@ InitApp(
 
     if (g_pLoaderWnd->CreateWnd())
     {
-        // Go to the bottom of the hell
+        // Go to the bottom of the (s)hell
         ::SetWindowPos(g_pLoaderWnd->m_hWnd, HWND_BOTTOM, 0, 0, 0, 0,
                        SWP_NOACTIVATE | SWP_NOMOVE | SWP_NOSIZE);
     }
@@ -236,7 +236,7 @@ InitApp(
     if (!g_bOnWow64)
         GetPopupTipbar(g_pLoaderWnd->m_hWnd, g_fWinLogon);
 
-    // Do x64 stuffs
+    // Initialize x64-specific support
     CheckX64System(lpCmdLine);
 
     return TRUE;
@@ -264,7 +264,7 @@ DoMainLoop(VOID)
 
     if (g_bOnWow64) // Is the current process on WoW64?
     {
-        // Just a simple message loop
+        // Simply run a message loop
         while (::GetMessage(&msg, NULL, 0, 0))
         {
             ::TranslateMessage(&msg);
@@ -273,26 +273,30 @@ DoMainLoop(VOID)
         return (INT)msg.wParam;
     }
 
-    // Open the existing event by the name
-    HANDLE hSwitchEvent = ::OpenEvent(SYNCHRONIZE, FALSE, TEXT("WinSta0_DesktopSwitch"));
-
-    // The target events to watch
-    HANDLE ahEvents[WATCHENTRY_MAX + 1];
+    // We wait on the CRegWatcher and the WinSta0 desktop-switch events.
+    HANDLE ahEvents[WI_REGEVTS_MAX + 1];
+    DWORD dwEventCount;
 
-    // Borrow some handles from CRegWatcher
-    CopyMemory(ahEvents, CRegWatcher::s_ahWatchEvents, WATCHENTRY_MAX * sizeof(HANDLE));
+    // Get the CRegWatcher handles
+    CopyMemory(ahEvents, CRegWatcher::s_ahWatchEvents, WI_REGEVTS_MAX * sizeof(HANDLE));
+    dwEventCount = WI_REGEVTS_MAX;
 
-    ahEvents[WI_DESKTOP_SWITCH] = hSwitchEvent; // Add it
+    // Open the WinSta0 desktop-switch event and add it
+    HANDLE hSwitchEvent = ::OpenEvent(SYNCHRONIZE, FALSE, TEXT("WinSta0_DesktopSwitch"));
+    if (hSwitchEvent)
+    {
+        ahEvents[WI_DESKTOP_SWITCH] = hSwitchEvent;
+        ++dwEventCount;
+    }
 
-    // Another message loop
+    // Run the event loop
     for (;;)
     {
-        // Wait for target signal
-        DWORD dwWait = ::MsgWaitForMultipleObjects(_countof(ahEvents), ahEvents, 0, INFINITE,
-                                                   QS_ALLINPUT);
-        if (dwWait == (WAIT_OBJECT_0 + _countof(ahEvents))) // Is input available?
+        DWORD dwWait = ::MsgWaitForMultipleObjects(dwEventCount, ahEvents,
+                                                   FALSE, INFINITE, QS_ALLINPUT);
+        if (dwWait == (WAIT_OBJECT_0 + dwEventCount)) // Is input available?
         {
-            // Do the events
+            // Pump available messages
             while (::PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
             {
                 if (msg.message == WM_QUIT)
@@ -307,14 +311,30 @@ DoMainLoop(VOID)
             SetGlobalCompartmentDWORD(GUID_COMPARTMENT_SPEECH_OPENCLOSE, 0);
             ::ResetEvent(hSwitchEvent);
         }
-        else // Do the other events
+        else if (dwWait < (WAIT_OBJECT_0 + WI_REGEVTS_MAX)) // Do the other events
         {
             CRegWatcher::OnEvent(dwWait - WAIT_OBJECT_0);
         }
+        else if (dwWait == WAIT_TIMEOUT)
+        {
+            // Ignore
+#ifndef NDEBUG
+            OutputDebugStringA("DoMainLoop: WAIT_TIMEOUT\n");
+#endif
+        }
+        else if (dwWait == WAIT_FAILED)
+        {
+            // Something failed, bail out
+#ifndef NDEBUG
+            OutputDebugStringA("DoMainLoop: WAIT_FAILED, exiting\n");
+#endif
+            break;
+        }
     }
 
 Quit:
-    ::CloseHandle(hSwitchEvent);
+    if (hSwitchEvent)
+        ::CloseHandle(hSwitchEvent);
 
     return (INT)msg.wParam;
 }

base/ctf/ctfmon/precomp.h

@@ -32,17 +32,18 @@ VOID UninitApp(VOID);
 
 typedef enum WATCH_INDEX
 {
-    WI_TOGGLE            = 0,
-    WI_MACHINE_TIF       = 1,
-    WI_PRELOAD           = 2,
-    WI_RUN               = 3,
-    WI_USER_TIF          = 4,
-    WI_USER_SPEECH       = 5,
-    WI_APPEARANCE        = 6,
-    WI_COLORS            = 7,
-    WI_WINDOW_METRICS    = 8,
-    WI_MACHINE_SPEECH    = 9,
-    WI_KEYBOARD_LAYOUT   = 10,
-    WI_ASSEMBLIES        = 11,
-    WI_DESKTOP_SWITCH    = 12,
+    WI_TOGGLE           = 0,
+    WI_MACHINE_TIF      = 1,
+    WI_PRELOAD          = 2,
+    WI_RUN              = 3,
+    WI_USER_TIF         = 4,
+    WI_USER_SPEECH      = 5,
+    WI_APPEARANCE       = 6,
+    WI_COLORS           = 7,
+    WI_WINDOW_METRICS   = 8,
+    WI_MACHINE_SPEECH   = 9,
+    WI_KEYBOARD_LAYOUT  = 10,
+    WI_ASSEMBLIES       = 11,
+    WI_REGEVTS_MAX,
+    WI_DESKTOP_SWITCH   = WI_REGEVTS_MAX,
 } WATCH_INDEX;