Jinoh Kang (@iamahuman) commented about dlls/win32u/winstation.c:
+/* return a locked session object for a thread id and type */ +static BOOL get_thread_session_object( UINT tid, enum object_type type, struct object_info *info, + struct object_lock *lock ) +{ + struct shared_session *session; + BOOL valid = TRUE; + + TRACE( "tid %04x, type %u\n", tid, type ); + + memset( info, 0, sizeof(*info) ); + info->index = -1; + + while ((session = get_shared_session( !valid ))) + { + info->session_id = session->id; + if ((info->index = get_thread_session_object_index( tid, type, &info->id )) == -1) break; Suppose that:
1. `get_thread_session_object_index` succeeded in the previous iteration, but 2. `get_thread_session_object_index` failed in the current iteration. Then, `info->id` has been set to nonzero value due to (1), but `info->index` is set to `-1` due to (2). On the next `get_shared_desktop()` call, `try_lock_session_object()` will notice the nonzero id, and take the address of `objects[-1]` which is out-of-bounds. --- You have two ways to address this, depending on how you want to treat the `info` parameter: | Postcondition on failure | Changes to callee (`get_thread_session_object`) | Changes to caller (`get_shared_desktop`) | |--------------------------|----------------------------------------|----------| | `info` is clobbered | (No changes) | **Don't pass a persistent structure via the `info` parameter.** | | `info` is zeroed | **Zero `info` before returning `FALSE`.** | (No changes) | In general, if you want to handle errors gracefully, you have to do it seriously or not at all. It's essential to define a clear code contract[^1][^2] even if you're not putting it into words. If this seems like too much work, you should just crash on failure instead of introducing subtle failure modes. [^1]: [Design by contract - Wikipedia](<https://en.wikipedia.org/wiki/Design_by_contract>) [^2]: C# example: [Code Contracts - .NET Framework | Microsoft Learn](<https://learn.microsoft.com/en-us/dotnet/framework/debug-trace-profile/code-contracts>) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_64975