[PATCH 0/1] MR10648: sechost: Catch invalid HDEVNOTIFY arguments to I_ScUnregisterDeviceNotification.

[Tim Clem (＠tclem)]

Steam has started unregistering an invalid handle on shutdown, resulting in a crash. On native, invalid handles are detected. I independently tested that these errors are caught at the sechost level, not just in user32.

Perhaps the magic value is overkill, but we do it in some similar situations and it felt better than attempting to immediately dive into the list entry.

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

[PATCH 1/1] sechost: Catch invalid HDEVNOTIFY arguments to I_ScUnregisterDeviceNotification.

[Tim Clem]

From: Tim Clem <[e-mail]>

---
 dlls/sechost/service.c    | 34 ++++++++++++++++++++++++++++++----
 dlls/user32/tests/input.c | 13 +++++++++++--
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/dlls/sechost/service.c b/dlls/sechost/service.c
index 474d977ac5e..4211ab36a4d 100644
--- a/dlls/sechost/service.c
+++ b/dlls/sechost/service.c
@@ -1979,8 +1979,11 @@ BOOL WINAPI DECLSPEC_HOTPATCH StartServiceCtrlDispatcherW( const SERVICE_TABLE_E
 static HANDLE device_notify_thread;
 static struct list device_notify_list = LIST_INIT(device_notify_list);
 
+#define DEVICE_NOTIFY_MAGIC 0xdecafbad
+
 struct device_notify
 {
+    DWORD magic;
     struct list entry;
     WCHAR *path;
     HANDLE handle;
@@ -1995,6 +1998,7 @@ static struct device_notify *device_notify_copy( struct device_notify *notify, D
     struct device_notify *event;
 
     if (!(event = calloc( 1, sizeof(*event) + header->dbch_size ))) return NULL;
+    event->magic = DEVICE_NOTIFY_MAGIC;
     event->handle = notify->handle;
     event->callback = notify->callback;
     memcpy( event->header, header, header->dbch_size );
@@ -2143,6 +2147,7 @@ HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( HANDLE handle, DEV_BROADCAST_H
         SetLastError(ERROR_NOT_ENOUGH_MEMORY);
         return NULL;
     }
+    notify->magic = DEVICE_NOTIFY_MAGIC;
     notify->handle = handle;
     notify->callback = callback;
     memcpy( notify->header, filter, filter->dbch_size );
@@ -2183,16 +2188,37 @@ HDEVNOTIFY WINAPI I_ScRegisterDeviceNotification( HANDLE handle, DEV_BROADCAST_H
 BOOL WINAPI I_ScUnregisterDeviceNotification( HDEVNOTIFY handle )
 {
     struct device_notify *notify = handle;
+    BOOL ret = TRUE;
 
     TRACE("%p\n", handle);
 
     if (!handle)
+    {
+        SetLastError( ERROR_INVALID_HANDLE );
         return FALSE;
+    }
 
     EnterCriticalSection( &service_cs );
-    list_remove( ¬ify->entry );
+
+    __TRY
+    {
+        if (notify->magic != DEVICE_NOTIFY_MAGIC)
+        {
+            SetLastError( ERROR_INVALID_HANDLE );
+            ret = FALSE;
+        }
+
+        list_remove( ¬ify->entry );
+        free( notify->path );
+        free( notify );
+    }
+    __EXCEPT_PAGE_FAULT
+    {
+        SetLastError( ERROR_INVALID_HANDLE );
+        ret = FALSE;
+    }
+    __ENDTRY
+
     LeaveCriticalSection(&service_cs);
-    free( notify->path );
-    free( notify );
-    return TRUE;
+    return ret;
 }
diff --git a/dlls/user32/tests/input.c b/dlls/user32/tests/input.c
index 905ff0dd922..cf3441c285a 100644
--- a/dlls/user32/tests/input.c
+++ b/dlls/user32/tests/input.c
@@ -5250,8 +5250,17 @@ static void test_input_message_source(void)
 
 static void test_UnregisterDeviceNotification(void)
 {
-    BOOL ret = UnregisterDeviceNotification(NULL);
-    ok(ret == FALSE, "Unregistering NULL Device Notification returned: %d\n", ret);
+    BOOL ret;
+
+    SetLastError( 0xdeadbeef );
+    ret = UnregisterDeviceNotification( NULL );
+    ok( ret == FALSE, "Unregistering NULL Device Notification returned: %d\n", ret );
+    ok_ret( ERROR_INVALID_HANDLE, GetLastError() );
+
+    SetLastError( 0xdeadbeef );
+    ret = UnregisterDeviceNotification( (HDEVNOTIFY)0xdeadbeef );
+    ok( ret == FALSE, "Unregistering invalid HDEVNOTIFY returned: %d\n", ret );
+    ok_ret( ERROR_INVALID_HANDLE, GetLastError() );
 }
 
 static void test_SendInput( WORD vkey, WCHAR wch, HKL hkl )
-- 
GitLab

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