[MSGINA][WINLOGON] Carefully zero password memory buffers before freeing them (#8172)

Including variables containing pointers to a password buffer and its lengths.
This commit is contained in:
Hermès Bélusca-Maïto
2025-06-19 17:32:43 +02:00
parent b087c08d76
commit f60be66d1f
4 changed files with 57 additions and 15 deletions

View File

@@ -541,9 +541,19 @@ FreeWlxMprInfo(
if (MprNotifyInfo->pszDomain)
LocalFree(MprNotifyInfo->pszDomain);
if (MprNotifyInfo->pszPassword)
{
/* Zero out the password buffer before freeing it */
SIZE_T pwdLen = (wcslen(MprNotifyInfo->pszPassword) + 1) * sizeof(WCHAR);
SecureZeroMemory(MprNotifyInfo->pszPassword, pwdLen);
LocalFree(MprNotifyInfo->pszPassword);
}
if (MprNotifyInfo->pszOldPassword)
{
/* Zero out the password buffer before freeing it */
SIZE_T pwdLen = (wcslen(MprNotifyInfo->pszOldPassword) + 1) * sizeof(WCHAR);
SecureZeroMemory(MprNotifyInfo->pszOldPassword, pwdLen);
LocalFree(MprNotifyInfo->pszOldPassword);
}
}
static

View File

@@ -622,7 +622,7 @@ DoChangePassword(
MB_OK | MB_ICONEXCLAMATION,
IDS_CHANGEPWDTITLE,
IDS_NONMATCHINGPASSWORDS);
return FALSE;
goto done;
}
/* Calculate the request buffer size */
@@ -639,7 +639,7 @@ DoChangePassword(
if (RequestBuffer == NULL)
{
ERR("HeapAlloc failed\n");
return FALSE;
goto done;
}
/* Initialize the request buffer */
@@ -729,13 +729,27 @@ DoChangePassword(
(wcscmp(Domain, pgContext->DomainName) == 0) &&
(wcscmp(OldPassword, pgContext->Password) == 0))
{
ZeroMemory(pgContext->Password, sizeof(pgContext->Password));
SecureZeroMemory(pgContext->Password, sizeof(pgContext->Password));
wcscpy(pgContext->Password, NewPassword1);
}
done:
/* Zero out the password buffers */
SecureZeroMemory(&OldPassword, sizeof(OldPassword));
SecureZeroMemory(&NewPassword1, sizeof(NewPassword1));
SecureZeroMemory(&NewPassword2, sizeof(NewPassword2));
if (RequestBuffer != NULL)
{
/* Zero out the password buffers before freeing them */
SecureZeroMemory(RequestBuffer->OldPassword.Buffer,
RequestBuffer->OldPassword.MaximumLength);
SecureZeroMemory(&RequestBuffer->OldPassword, sizeof(RequestBuffer->OldPassword));
SecureZeroMemory(RequestBuffer->NewPassword.Buffer,
RequestBuffer->NewPassword.MaximumLength);
SecureZeroMemory(&RequestBuffer->NewPassword, sizeof(RequestBuffer->NewPassword));
HeapFree(GetProcessHeap(), 0, RequestBuffer);
}
if (ResponseBuffer != NULL)
LsaFreeReturnBuffer(ResponseBuffer);
@@ -1143,20 +1157,23 @@ DoLogon(
(SubStatus == STATUS_PASSWORD_EXPIRED))
{
if (SubStatus == STATUS_PASSWORD_MUST_CHANGE)
{
ResourceMessageBox(pgContext,
hwndDlg,
MB_OK | MB_ICONSTOP,
IDS_LOGONTITLE,
IDS_PASSWORDMUSTCHANGE);
}
else
{
ResourceMessageBox(pgContext,
hwndDlg,
MB_OK | MB_ICONSTOP,
IDS_LOGONTITLE,
IDS_PASSWORDEXPIRED);
}
if (!OnChangePassword(hwndDlg,
pgContext))
if (!OnChangePassword(hwndDlg, pgContext))
goto done;
Status = DoLoginTasks(pgContext,
@@ -1167,7 +1184,6 @@ DoLogon(
if (!NT_SUCCESS(Status))
{
TRACE("Login after password change failed! (Status 0x%08lx)\n", Status);
goto done;
}
}
@@ -1220,7 +1236,7 @@ DoLogon(
goto done;
}
ZeroMemory(pgContext->Password, sizeof(pgContext->Password));
SecureZeroMemory(pgContext->Password, sizeof(pgContext->Password));
wcscpy(pgContext->Password, Password);
result = TRUE;
@@ -1232,7 +1248,12 @@ done:
HeapFree(GetProcessHeap(), 0, UserName);
if (Password != NULL)
{
/* Zero out the password buffer before freeing it */
SIZE_T pwdLen = (wcslen(Password) + 1) * sizeof(WCHAR);
SecureZeroMemory(Password, pwdLen);
HeapFree(GetProcessHeap(), 0, Password);
}
if (Domain != NULL)
HeapFree(GetProcessHeap(), 0, Domain);
@@ -1542,7 +1563,12 @@ DoUnlock(
HeapFree(GetProcessHeap(), 0, UserName);
if (Password != NULL)
{
/* Zero out the password buffer before freeing it */
SIZE_T pwdLen = (wcslen(Password) + 1) * sizeof(WCHAR);
SecureZeroMemory(Password, pwdLen);
HeapFree(GetProcessHeap(), 0, Password);
}
return res;
}

View File

@@ -130,9 +130,11 @@ MyLogonUser(
AuthInfo->UserName.MaximumLength = UserName.MaximumLength;
AuthInfo->UserName.Buffer = (PWCHAR)Ptr;
if (UserName.MaximumLength > 0)
{
RtlCopyMemory(AuthInfo->UserName.Buffer,
UserName.Buffer,
UserName.MaximumLength);
}
Ptr += UserName.MaximumLength;
@@ -140,9 +142,11 @@ MyLogonUser(
AuthInfo->Password.MaximumLength = Password.MaximumLength;
AuthInfo->Password.Buffer = (PWCHAR)Ptr;
if (Password.MaximumLength > 0)
{
RtlCopyMemory(AuthInfo->Password.Buffer,
Password.Buffer,
Password.MaximumLength);
}
/* Create the Logon SID*/
AllocateLocallyUniqueId(&LogonId);
@@ -232,21 +236,18 @@ MyLogonUser(
TRACE("Luid: 0x%08lx%08lx\n", Luid.HighPart, Luid.LowPart);
if (TokenHandle != NULL)
{
TRACE("TokenHandle: %p\n", TokenHandle);
}
*phToken = TokenHandle;
done:
SecureZeroMemory(&Password, sizeof(Password));
if (ProfileBuffer != NULL)
LsaFreeReturnBuffer(ProfileBuffer);
if (!NT_SUCCESS(Status))
{
if (TokenHandle != NULL)
CloseHandle(TokenHandle);
}
if (!NT_SUCCESS(Status) && (TokenHandle != NULL))
CloseHandle(TokenHandle);
if (TokenGroups != NULL)
RtlFreeHeap(RtlGetProcessHeap(), 0, TokenGroups);
@@ -258,7 +259,12 @@ done:
RtlFreeSid(LogonSid);
if (AuthInfo != NULL)
{
/* Zero out the password buffers before freeing */
SecureZeroMemory(AuthInfo->Password.Buffer, AuthInfo->Password.MaximumLength);
SecureZeroMemory(&AuthInfo->Password, sizeof(AuthInfo->Password));
RtlFreeHeap(RtlGetProcessHeap(), 0, AuthInfo);
}
return Status;
}

View File

@@ -1071,7 +1071,7 @@ WlxLogoff(
TRACE("WlxLogoff(%p)\n", pWlxContext);
/* Delete the password */
ZeroMemory(pgContext->Password, sizeof(pgContext->Password));
SecureZeroMemory(pgContext->Password, sizeof(pgContext->Password));
/* Close the user token */
CloseHandle(pgContext->UserToken);