From b254ea8274b7edd744ef661a8b8489873ac2dd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Thu, 19 Jun 2025 17:12:27 +0200 Subject: [PATCH] [MSGINA][WINLOGON] Perform thorough memory cleanup after user logging (#8155) - MSGINA: The `pMprNotifyInfo` and `pProfile` structures returned by `WlxLoggedOutSAS()`, as well as all of their pointer fields, are allocated by `LocalAlloc()`[^1][^2]. This is what Windows' Winlogon expects (and ours too, for interoperability with GINA dlls written for Windows), as it then frees them using `LocalFree()`. - WINLOGON: In `HandleLogon()`, free the cached `MprNotifyInfo` and `Profile` buffers (and all their members) obtained from a previous call to `WlxLoggedOutSAS()`. [^1]: https://learn.microsoft.com/en-us/windows/win32/api/winwlx/nf-winwlx-wlxloggedoutsas [^2]: 3rd-party GINAs rely on this as well. One example can be found at: https://www.codeproject.com/Articles/20656/Winlogon-using-Mobile-Disk --- base/system/winlogon/sas.c | 64 +++++++++++++++++++++++++++++++-- base/system/winlogon/winlogon.h | 2 +- dll/win32/msgina/msgina.c | 42 +++++++++++----------- 3 files changed, 83 insertions(+), 25 deletions(-) diff --git a/base/system/winlogon/sas.c b/base/system/winlogon/sas.c index 9ea8c426a3b..813f2519bf5 100644 --- a/base/system/winlogon/sas.c +++ b/base/system/winlogon/sas.c @@ -491,6 +491,61 @@ quit: RevertToSelf(); } +/** + * @brief + * Frees the Profile information structure (WLX_PROFILE_V1_0 + * or WLX_PROFILE_V2_0) allocated by the GINA. + **/ +static VOID +FreeWlxProfileInfo( + _Inout_ PVOID Profile) +{ + PWLX_PROFILE_V2_0 pProfile = (PWLX_PROFILE_V2_0)Profile; + + if (pProfile->dwType != WLX_PROFILE_TYPE_V1_0 + && pProfile->dwType != WLX_PROFILE_TYPE_V2_0) + { + ERR("WL: Wrong profile info\n"); + return; + } + + if (pProfile->pszProfile) + LocalFree(pProfile->pszProfile); + if (pProfile->dwType >= WLX_PROFILE_TYPE_V2_0) + { + if (pProfile->pszPolicy) + LocalFree(pProfile->pszPolicy); + if (pProfile->pszNetworkDefaultUserProfile) + LocalFree(pProfile->pszNetworkDefaultUserProfile); + if (pProfile->pszServerName) + LocalFree(pProfile->pszServerName); + if (pProfile->pszEnvironment) + LocalFree(pProfile->pszEnvironment); + } +} + +/** + * @brief + * Frees the MPR information structure allocated by the GINA. + * + * @note + * Currently used only in HandleLogon(), but will also be used + * by WlxChangePasswordNotify(Ex) once implemented. + **/ +static VOID +FreeWlxMprInfo( + _Inout_ PWLX_MPR_NOTIFY_INFO MprNotifyInfo) +{ + if (MprNotifyInfo->pszUserName) + LocalFree(MprNotifyInfo->pszUserName); + if (MprNotifyInfo->pszDomain) + LocalFree(MprNotifyInfo->pszDomain); + if (MprNotifyInfo->pszPassword) + LocalFree(MprNotifyInfo->pszPassword); + if (MprNotifyInfo->pszOldPassword) + LocalFree(MprNotifyInfo->pszOldPassword); +} + static BOOL HandleLogon( @@ -611,10 +666,13 @@ HandleLogon( cleanup: if (Session->Profile) { - HeapFree(GetProcessHeap(), 0, Session->Profile->pszProfile); - HeapFree(GetProcessHeap(), 0, Session->Profile); + FreeWlxProfileInfo(Session->Profile); + LocalFree(Session->Profile); + Session->Profile = NULL; } - Session->Profile = NULL; + FreeWlxMprInfo(&Session->MprNotifyInfo); + ZeroMemory(&Session->MprNotifyInfo, sizeof(Session->MprNotifyInfo)); + if (!ret && ProfileInfo.hProfile != INVALID_HANDLE_VALUE) { UnloadUserProfile(Session->UserToken, ProfileInfo.hProfile); diff --git a/base/system/winlogon/winlogon.h b/base/system/winlogon/winlogon.h index 9954230030d..7e4609c1576 100644 --- a/base/system/winlogon/winlogon.h +++ b/base/system/winlogon/winlogon.h @@ -254,7 +254,7 @@ typedef struct _WLSESSION /* Logon informations */ DWORD Options; WLX_MPR_NOTIFY_INFO MprNotifyInfo; - WLX_PROFILE_V2_0 *Profile; + PWLX_PROFILE_V2_0 Profile; } WLSESSION, *PWLSESSION; typedef enum _NOTIFICATION_TYPE diff --git a/dll/win32/msgina/msgina.c b/dll/win32/msgina/msgina.c index cdf796a0a13..9c4a71915d3 100644 --- a/dll/win32/msgina/msgina.c +++ b/dll/win32/msgina/msgina.c @@ -834,10 +834,10 @@ CreateProfile( IN PWSTR Domain, IN PWSTR Password) { - LPWSTR ProfilePath = NULL; - LPWSTR lpEnvironment = NULL; - TOKEN_STATISTICS Stats; PWLX_PROFILE_V2_0 pProfile = NULL; + PWSTR pProfilePath = NULL; + PWSTR pEnvironment = NULL; + TOKEN_STATISTICS Stats; DWORD cbStats, cbSize; DWORD dwLength; BOOL bResult; @@ -862,13 +862,13 @@ CreateProfile( bResult = GetProfilesDirectoryW(NULL, &cbSize); if (!bResult && GetLastError() == ERROR_INSUFFICIENT_BUFFER) { - ProfilePath = HeapAlloc(GetProcessHeap(), 0, cbSize * sizeof(WCHAR)); - if (!ProfilePath) + pProfilePath = LocalAlloc(LMEM_FIXED, cbSize * sizeof(WCHAR)); + if (!pProfilePath) { - WARN("HeapAlloc() failed\n"); + WARN("LocalAlloc() failed\n"); goto cleanup; } - bResult = GetProfilesDirectoryW(ProfilePath, &cbSize); + bResult = GetProfilesDirectoryW(pProfilePath, &cbSize); } if (!bResult) { @@ -877,30 +877,30 @@ CreateProfile( } /* Allocate memory for profile */ - pProfile = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(WLX_PROFILE_V2_0)); + pProfile = LocalAlloc(LMEM_FIXED | LMEM_ZEROINIT, sizeof(*pProfile)); if (!pProfile) { WARN("HeapAlloc() failed\n"); goto cleanup; } pProfile->dwType = WLX_PROFILE_TYPE_V2_0; - pProfile->pszProfile = ProfilePath; + pProfile->pszProfile = pProfilePath; cbSize = sizeof(L"LOGONSERVER=\\\\") + wcslen(pgContext->DomainName) * sizeof(WCHAR) + sizeof(UNICODE_NULL); - lpEnvironment = HeapAlloc(GetProcessHeap(), 0, cbSize); - if (!lpEnvironment) + pEnvironment = LocalAlloc(LMEM_FIXED, cbSize); + if (!pEnvironment) { - WARN("HeapAlloc() failed\n"); + WARN("LocalAlloc() failed\n"); goto cleanup; } - StringCbPrintfW(lpEnvironment, cbSize, L"LOGONSERVER=\\\\%ls", pgContext->DomainName); - ASSERT(wcslen(lpEnvironment) == cbSize / sizeof(WCHAR) - 2); - lpEnvironment[cbSize / sizeof(WCHAR) - 1] = UNICODE_NULL; + StringCbPrintfW(pEnvironment, cbSize, L"LOGONSERVER=\\\\%ls", pgContext->DomainName); + ASSERT(wcslen(pEnvironment) == cbSize / sizeof(WCHAR) - 2); + pEnvironment[cbSize / sizeof(WCHAR) - 1] = UNICODE_NULL; - pProfile->pszEnvironment = lpEnvironment; + pProfile->pszEnvironment = pEnvironment; if (!GetTokenInformation(pgContext->UserToken, TokenStatistics, @@ -922,12 +922,12 @@ CreateProfile( return TRUE; cleanup: + if (pEnvironment) + LocalFree(pEnvironment); + if (pProfilePath) + LocalFree(pProfilePath); if (pProfile) - { - HeapFree(GetProcessHeap(), 0, pProfile->pszEnvironment); - } - HeapFree(GetProcessHeap(), 0, pProfile); - HeapFree(GetProcessHeap(), 0, ProfilePath); + LocalFree(pProfile); return FALSE; }