[reactos] 03/04: [MSV1_0] Extend parameter validation in LsaApLogonUserEx2() and MsvpChangePassword(); fix crash in LsaApLogonUserEx2() when LogonDomainName points to a NULL string.
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8210396cb42420abc57f12...
commit 8210396cb42420abc57f121d0c278dee8fbfc671 Author: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org AuthorDate: Tue Jun 18 02:27:08 2019 +0200 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org CommitDate: Thu Jan 2 22:16:58 2020 +0100
[MSV1_0] Extend parameter validation in LsaApLogonUserEx2() and MsvpChangePassword(); fix crash in LsaApLogonUserEx2() when LogonDomainName points to a NULL string. --- dll/win32/msv1_0/msv1_0.c | 130 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 116 insertions(+), 14 deletions(-)
diff --git a/dll/win32/msv1_0/msv1_0.c b/dll/win32/msv1_0/msv1_0.c index 39360caa38d..b7cdf46d9aa 100644 --- a/dll/win32/msv1_0/msv1_0.c +++ b/dll/win32/msv1_0/msv1_0.c @@ -496,6 +496,7 @@ MsvpChangePassword(IN PLSA_CLIENT_REQUEST ClientRequest, OUT PULONG ReturnBufferLength, OUT PNTSTATUS ProtocolStatus) { + NTSTATUS Status; PMSV1_0_CHANGEPASSWORD_REQUEST RequestBuffer; ULONG_PTR PtrOffset;
@@ -506,7 +507,6 @@ MsvpChangePassword(IN PLSA_CLIENT_REQUEST ClientRequest, RPC_UNICODE_STRING Names[1]; SAMPR_ULONG_ARRAY RelativeIds = {0, NULL}; SAMPR_ULONG_ARRAY Use = {0, NULL}; - NTSTATUS Status;
ENCRYPTED_NT_OWF_PASSWORD OldNtPassword; ENCRYPTED_NT_OWF_PASSWORD NewNtPassword; @@ -524,17 +524,48 @@ MsvpChangePassword(IN PLSA_CLIENT_REQUEST ClientRequest, PENCRYPTED_LM_OWF_PASSWORD pOldLmEncryptedWithNewLm = NULL; PENCRYPTED_LM_OWF_PASSWORD pNewLmEncryptedWithOldLm = NULL;
- TRACE("()\n"); + TRACE("MsvpChangePassword()\n"); + + /* Parameters validation */ + + if (SubmitBufferLength < sizeof(MSV1_0_CHANGEPASSWORD_REQUEST)) + { + ERR("Invalid SubmitBufferLength %lu\n", SubmitBufferLength); + return STATUS_INVALID_PARAMETER; + }
RequestBuffer = (PMSV1_0_CHANGEPASSWORD_REQUEST)ProtocolSubmitBuffer;
/* Fix-up pointers in the request buffer info */ PtrOffset = (ULONG_PTR)ProtocolSubmitBuffer - (ULONG_PTR)ClientBufferBase;
+ Status = RtlValidateUnicodeString(0, &RequestBuffer->DomainName); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. RequestBuffer->DomainName.Buffer = FIXUP_POINTER(RequestBuffer->DomainName.Buffer, PtrOffset); + RequestBuffer->DomainName.MaximumLength = RequestBuffer->DomainName.Length; + + Status = RtlValidateUnicodeString(0, &RequestBuffer->AccountName); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. RequestBuffer->AccountName.Buffer = FIXUP_POINTER(RequestBuffer->AccountName.Buffer, PtrOffset); + RequestBuffer->AccountName.MaximumLength = RequestBuffer->AccountName.Length; + + Status = RtlValidateUnicodeString(0, &RequestBuffer->OldPassword); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. RequestBuffer->OldPassword.Buffer = FIXUP_POINTER(RequestBuffer->OldPassword.Buffer, PtrOffset); + RequestBuffer->OldPassword.MaximumLength = RequestBuffer->OldPassword.Length; + + Status = RtlValidateUnicodeString(0, &RequestBuffer->NewPassword); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. RequestBuffer->NewPassword.Buffer = FIXUP_POINTER(RequestBuffer->NewPassword.Buffer, PtrOffset); + RequestBuffer->NewPassword.MaximumLength = RequestBuffer->NewPassword.Length;
TRACE("Domain: %S\n", RequestBuffer->DomainName.Buffer); TRACE("Account: %S\n", RequestBuffer->AccountName.Buffer); @@ -942,15 +973,15 @@ LsaApCallPackage(IN PLSA_CLIENT_REQUEST ClientRequest, OUT PULONG ReturnBufferLength, OUT PNTSTATUS ProtocolStatus) { - ULONG MessageType; NTSTATUS Status; + MSV1_0_PROTOCOL_MESSAGE_TYPE MessageType;
TRACE("LsaApCallPackage()\n");
if (SubmitBufferLength < sizeof(MSV1_0_PROTOCOL_MESSAGE_TYPE)) return STATUS_INVALID_PARAMETER;
- MessageType = (ULONG)*((PMSV1_0_PROTOCOL_MESSAGE_TYPE)ProtocolSubmitBuffer); + MessageType = *((PMSV1_0_PROTOCOL_MESSAGE_TYPE)ProtocolSubmitBuffer);
*ProtocolReturnBuffer = NULL; *ReturnBufferLength = 0; @@ -1140,6 +1171,11 @@ LsaApLogonUserEx2(IN PLSA_CLIENT_REQUEST ClientRequest, OUT PSECPKG_PRIMARY_CRED PrimaryCredentials, /* Not supported yet */ OUT PSECPKG_SUPPLEMENTAL_CRED_ARRAY *SupplementalCredentials) /* Not supported yet */ { + static const UNICODE_STRING NtAuthorityU = RTL_CONSTANT_STRING(L"NT AUTHORITY"); + static const UNICODE_STRING LocalServiceU = RTL_CONSTANT_STRING(L"LocalService"); + static const UNICODE_STRING NetworkServiceU = RTL_CONSTANT_STRING(L"NetworkService"); + + NTSTATUS Status; PMSV1_0_INTERACTIVE_LOGON LogonInfo; WCHAR ComputerName[MAX_COMPUTERNAME_LENGTH + 1]; SAMPR_HANDLE ServerHandle = NULL; @@ -1157,9 +1193,8 @@ LsaApLogonUserEx2(IN PLSA_CLIENT_REQUEST ClientRequest, LARGE_INTEGER PasswordLastSet; DWORD ComputerNameSize; BOOL SpecialAccount = FALSE; - NTSTATUS Status;
- TRACE("LsaApLogonUser()\n"); + TRACE("LsaApLogonUserEx2()\n");
TRACE("LogonType: %lu\n", LogonType); TRACE("ProtocolSubmitBuffer: %p\n", ProtocolSubmitBuffer); @@ -1171,40 +1206,107 @@ LsaApLogonUserEx2(IN PLSA_CLIENT_REQUEST ClientRequest, *AccountName = NULL; *AuthenticatingAuthority = NULL;
+ /* Parameters validation */ if (LogonType == Interactive || LogonType == Batch || LogonType == Service) { ULONG_PTR PtrOffset;
+ if (SubmitBufferSize < sizeof(MSV1_0_INTERACTIVE_LOGON)) + { + ERR("Invalid SubmitBufferSize %lu\n", SubmitBufferSize); + return STATUS_INVALID_PARAMETER; + } + LogonInfo = (PMSV1_0_INTERACTIVE_LOGON)ProtocolSubmitBuffer;
+ if (LogonInfo->MessageType != MsV1_0InteractiveLogon && + LogonInfo->MessageType != MsV1_0WorkstationUnlockLogon) + { + ERR("Invalid MessageType %lu\n", LogonInfo->MessageType); + return STATUS_BAD_VALIDATION_CLASS; + } + +#if 0 // FIXME: These checks happen to be done on Windows. We however keep them general on ReactOS for now... + if (LogonInfo->UserName.Length > 512) // CRED_MAX_STRING_LENGTH * sizeof(WCHAR) or (CREDUI_MAX_USERNAME_LENGTH (== CRED_MAX_USERNAME_LENGTH) - 1) * sizeof(WCHAR) + { + ERR("UserName too long (%lu, maximum 512)\n", LogonInfo->UserName.Length); + return STATUS_NAME_TOO_LONG; + } + if (LogonInfo->Password.Length > 512) // CREDUI_MAX_PASSWORD_LENGTH * sizeof(WCHAR) + { + ERR("Password too long (%lu, maximum 512)\n", LogonInfo->Password.Length); + return STATUS_NAME_TOO_LONG; + } +#endif + /* Fix-up pointers in the authentication info */ PtrOffset = (ULONG_PTR)ProtocolSubmitBuffer - (ULONG_PTR)ClientBufferBase;
- LogonInfo->LogonDomainName.Buffer = FIXUP_POINTER(LogonInfo->LogonDomainName.Buffer, PtrOffset); + Status = RtlValidateUnicodeString(0, &LogonInfo->LogonDomainName); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + /* LogonDomainName is optional and can be an empty string */ + if (LogonInfo->LogonDomainName.Length) + { + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. + LogonInfo->LogonDomainName.Buffer = FIXUP_POINTER(LogonInfo->LogonDomainName.Buffer, PtrOffset); + LogonInfo->LogonDomainName.MaximumLength = LogonInfo->LogonDomainName.Length; + } + else + { + LogonInfo->LogonDomainName.Buffer = NULL; + LogonInfo->LogonDomainName.MaximumLength = 0; + } + + Status = RtlValidateUnicodeString(0, &LogonInfo->UserName); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + /* UserName is mandatory and cannot be an empty string */ + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. LogonInfo->UserName.Buffer = FIXUP_POINTER(LogonInfo->UserName.Buffer, PtrOffset); - LogonInfo->Password.Buffer = FIXUP_POINTER(LogonInfo->Password.Buffer, PtrOffset); + LogonInfo->UserName.MaximumLength = LogonInfo->UserName.Length; + + Status = RtlValidateUnicodeString(0, &LogonInfo->Password); + if (!NT_SUCCESS(Status)) + return STATUS_INVALID_PARAMETER; + /* Password is optional and can be an empty string */ + if (LogonInfo->Password.Length) + { + // TODO: Check for Buffer limits wrt. ClientBufferBase and alignment. + LogonInfo->Password.Buffer = FIXUP_POINTER(LogonInfo->Password.Buffer, PtrOffset); + LogonInfo->Password.MaximumLength = LogonInfo->Password.Length; + } + else + { + LogonInfo->Password.Buffer = NULL; + LogonInfo->Password.MaximumLength = 0; + }
TRACE("Domain: %S\n", LogonInfo->LogonDomainName.Buffer); TRACE("User: %S\n", LogonInfo->UserName.Buffer); TRACE("Password: %S\n", LogonInfo->Password.Buffer); + + // TODO: If LogonType == Service, do some extra work using LogonInfo->Password. } else { FIXME("LogonType %lu is not supported yet!\n", LogonType); return STATUS_NOT_IMPLEMENTED; } + // TODO: Add other LogonType validity checks.
/* Get the logon time */ NtQuerySystemTime(&LogonTime);
/* Get the computer name */ - ComputerNameSize = MAX_COMPUTERNAME_LENGTH + 1; + ComputerNameSize = ARRAYSIZE(ComputerName); GetComputerNameW(ComputerName, &ComputerNameSize);
/* Check for special accounts */ - if (_wcsicmp(LogonInfo->LogonDomainName.Buffer, L"NT AUTHORITY") == 0) + // FIXME: Windows does not do this that way!! (msv1_0 does not contain these hardcoded values) + if (RtlEqualUnicodeString(&LogonInfo->LogonDomainName, &NtAuthorityU, TRUE)) { SpecialAccount = TRUE;
@@ -1216,7 +1318,7 @@ LsaApLogonUserEx2(IN PLSA_CLIENT_REQUEST ClientRequest, return Status; }
- if (_wcsicmp(LogonInfo->UserName.Buffer, L"LocalService") == 0) + if (RtlEqualUnicodeString(&LogonInfo->UserName, &LocalServiceU, TRUE)) { TRACE("SpecialAccount: LocalService\n");
@@ -1235,7 +1337,7 @@ LsaApLogonUserEx2(IN PLSA_CLIENT_REQUEST ClientRequest, UserInfo->All.UserId = SECURITY_LOCAL_SERVICE_RID; UserInfo->All.PrimaryGroupId = SECURITY_LOCAL_SERVICE_RID; } - else if (_wcsicmp(LogonInfo->UserName.Buffer, L"NetworkService") == 0) + else if (RtlEqualUnicodeString(&LogonInfo->UserName, &NetworkServiceU, TRUE)) { TRACE("SpecialAccount: NetworkService\n");
@@ -1345,7 +1447,7 @@ LsaApLogonUserEx2(IN PLSA_CLIENT_REQUEST ClientRequest, /* Check the password */ if ((UserInfo->All.UserAccountControl & USER_PASSWORD_NOT_REQUIRED) == 0) { - Status = MsvpCheckPassword(&(LogonInfo->Password), + Status = MsvpCheckPassword(&LogonInfo->Password, UserInfo); if (!NT_SUCCESS(Status)) { @@ -1567,7 +1669,7 @@ done: Status = STATUS_LOGON_FAILURE; }
- TRACE("LsaApLogonUser done (Status 0x%08lx SubStatus 0x%08lx)\n", Status, *SubStatus); + TRACE("LsaApLogonUserEx2 done (Status 0x%08lx, SubStatus 0x%08lx)\n", Status, *SubStatus);
return Status; }
-
Hermès Bélusca-Maïto