[ros-diffs] [reactos] 03/04: [MSV1_0] Extend parameter validation in LsaApLogonUserEx2() and MsvpChangePassword(); fix crash in LsaApLogonUserEx2() when LogonDomainName points to a NULL string.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=8210396cb42420abc57f1…

commit 8210396cb42420abc57f121d0c278dee8fbfc671
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Tue Jun 18 02:27:08 2019 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)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;
 }