secur32: Split the NTLM credential and context handles into separate objects.

Robert Shearman rob at codeweavers.com
Tue Aug 7 11:12:32 CDT 2007


This involves using NegoHelper as the security context and a new 
NtlmCredentials structure as the credentials handle.

This prevents races with two threads using the helper object at the same 
time on two different context handles, eliminates the need to free the 
credential handle after freeing the context handles and also prevents a 
crash caused by not clearing session_key in DeleteSecurityContext.
---
  dlls/secur32/dispatcher.c   |    1
  dlls/secur32/ntlm.c         |  329 
+++++++++++++++++++++++++++----------------
  dlls/secur32/secur32_priv.h |    2
  3 files changed, 210 insertions(+), 122 deletions(-)
-------------- next part --------------
diff --git a/dlls/secur32/dispatcher.c b/dlls/secur32/dispatcher.c
index 71c8300..08b1b74 100644
--- a/dlls/secur32/dispatcher.c
+++ b/dlls/secur32/dispatcher.c
@@ -111,7 +111,6 @@ SECURITY_STATUS fork_helper(PNegoHelper 
     {
         *new_helper = helper;
         helper->major = helper->minor = helper->micro = -1;
-        helper->password = NULL;
         helper->com_buf = NULL;
         helper->com_buf_size = 0;
         helper->com_buf_offset = 0;
diff --git a/dlls/secur32/ntlm.c b/dlls/secur32/ntlm.c
index 890c74a..a1f6acc 100644
--- a/dlls/secur32/ntlm.c
+++ b/dlls/secur32/ntlm.c
@@ -40,6 +40,17 @@ #define MIN_NTLM_AUTH_MICRO_VERSION 25
 
 static CHAR ntlm_auth[] = "ntlm_auth";
 
+typedef struct _NtlmCredentials
+{
+    HelperMode mode;
+
+    /* these are all in the Unix codepage */
+    char *username_arg;
+    char *domain_arg;
+    char *password; /* not nul-terminated */
+    int pwlen;
+} NtlmCredentials, *PNtlmCredentials;
+
 /***********************************************************************
  *              QueryCredentialsAttributesA
  */
@@ -91,46 +102,35 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
  PVOID pGetKeyArgument, PCredHandle phCredential, PTimeStamp ptsExpiry)
 {
     SECURITY_STATUS ret;
-    PNegoHelper helper = NULL;
-    static CHAR server_helper_protocol[] = "--helper-protocol=squid-2.5-ntlmssp",
-                credentials_argv[] = "--use-cached-creds";
-
-    SEC_CHAR *client_user_arg = NULL;
-    SEC_CHAR *client_domain_arg = NULL;
+    PNtlmCredentials ntlm_cred = NULL;
     SEC_WCHAR *username = NULL, *domain = NULL;
 
-    SEC_CHAR *client_argv[6];
-    SEC_CHAR *server_argv[] = { ntlm_auth,
-        server_helper_protocol,
-        NULL };
-
     TRACE("(%s, %s, 0x%08x, %p, %p, %p, %p, %p, %p)\n",
      debugstr_w(pszPrincipal), debugstr_w(pszPackage), fCredentialUse,
      pLogonID, pAuthData, pGetKeyFn, pGetKeyArgument, phCredential, ptsExpiry);
 
-
     switch(fCredentialUse)
     {
         case SECPKG_CRED_INBOUND:
-            if( (ret = fork_helper(&helper, ntlm_auth, server_argv)) !=
-                    SEC_E_OK)
-            {
-                phCredential = NULL;
-                break;
-            }
+            ntlm_cred = HeapAlloc(GetProcessHeap(), 0, sizeof(*ntlm_cred));
+            if (!ntlm_cred)
+                ret = SEC_E_INSUFFICIENT_MEMORY;
             else
             {
-                helper->mode = NTLM_SERVER;
+                ntlm_cred->mode = NTLM_SERVER;
+                ntlm_cred->username_arg = NULL;
+                ntlm_cred->domain_arg = NULL;
+                ntlm_cred->password = NULL;
+                ntlm_cred->pwlen = 0;
                 phCredential->dwUpper = fCredentialUse;
-                phCredential->dwLower = (ULONG_PTR)helper;
+                phCredential->dwLower = (ULONG_PTR)ntlm_cred;
+                ret = SEC_E_OK;
             }
-            ret = SEC_E_OK;
             break;
         case SECPKG_CRED_OUTBOUND:
             {
                 static const char username_arg[] = "--username=";
                 static const char domain_arg[] = "--domain=";
-                static char helper_protocol[] = "--helper-protocol=ntlmssp-client-1";
                 int unixcp_size;
 
                 if(pAuthData == NULL)
@@ -176,67 +176,60 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
                            auth_data->DomainLength * sizeof(SEC_WCHAR));
                     domain[auth_data->DomainLength] = '\0';
                 }
+
+                ntlm_cred = HeapAlloc(GetProcessHeap(), 0, sizeof(*ntlm_cred));
+                if (!ntlm_cred)
+                {
+                    ret = SEC_E_INSUFFICIENT_MEMORY;
+                    break;
+                }
+                ntlm_cred->mode = NTLM_CLIENT;
+                ntlm_cred->password = NULL;
+                ntlm_cred->pwlen = 0;
+
                 TRACE("Username is %s\n", debugstr_w(username));
                 unixcp_size =  WideCharToMultiByte(CP_UNIXCP, WC_NO_BEST_FIT_CHARS,
                         username, -1, NULL, 0, NULL, NULL) + sizeof(username_arg);
-                client_user_arg = HeapAlloc(GetProcessHeap(), 0, unixcp_size);
-                lstrcpyA(client_user_arg, username_arg);
+                ntlm_cred->username_arg = HeapAlloc(GetProcessHeap(), 0, unixcp_size);
+                memcpy(ntlm_cred->username_arg, username_arg, sizeof(username_arg) - 1);
                 WideCharToMultiByte(CP_UNIXCP, WC_NO_BEST_FIT_CHARS, username, -1,
-                        client_user_arg + sizeof(username_arg) - 1, 
+                        ntlm_cred->username_arg + sizeof(username_arg) - 1, 
                         unixcp_size - sizeof(username_arg) + 1, NULL, NULL);
 
                 TRACE("Domain name is %s\n", debugstr_w(domain));
                 unixcp_size = WideCharToMultiByte(CP_UNIXCP, WC_NO_BEST_FIT_CHARS,
                         domain, -1, NULL, 0,  NULL, NULL) + sizeof(domain_arg);
-                client_domain_arg = HeapAlloc(GetProcessHeap(), 0, unixcp_size);
-                lstrcpyA(client_domain_arg, domain_arg);
+                ntlm_cred->domain_arg = HeapAlloc(GetProcessHeap(), 0, unixcp_size);
+                memcpy(ntlm_cred->domain_arg, domain_arg, sizeof(domain_arg) - 1);
                 WideCharToMultiByte(CP_UNIXCP, WC_NO_BEST_FIT_CHARS, domain,
-                        -1, client_domain_arg + sizeof(domain_arg) - 1, 
+                        -1, ntlm_cred->domain_arg + sizeof(domain_arg) - 1, 
                         unixcp_size - sizeof(domain) + 1, NULL, NULL);
 
-                client_argv[0] = ntlm_auth;
-                client_argv[1] = helper_protocol;
-                client_argv[2] = client_user_arg;
-                client_argv[3] = client_domain_arg;
-                client_argv[4] = credentials_argv;
-                client_argv[5] = NULL;
-
-                if((ret = fork_helper(&helper, ntlm_auth, client_argv)) !=
-                        SEC_E_OK)
+                if(pAuthData != NULL)
                 {
-                    phCredential = NULL;
-                    break;
-                }
-                else
-                {
-                    helper->mode = NTLM_CLIENT;
-
-                    if(pAuthData != NULL)
+                    PSEC_WINNT_AUTH_IDENTITY_W auth_data = 
+                    (PSEC_WINNT_AUTH_IDENTITY_W)pAuthData;
+                    
+                    if(auth_data->PasswordLength != 0)
                     {
-                        PSEC_WINNT_AUTH_IDENTITY_W auth_data = 
-                           (PSEC_WINNT_AUTH_IDENTITY_W)pAuthData;
-
-                        if(auth_data->PasswordLength != 0)
-                        {
-                            helper->pwlen = WideCharToMultiByte(CP_UNIXCP, 
-                                WC_NO_BEST_FIT_CHARS, auth_data->Password, 
-                                auth_data->PasswordLength, NULL, 0, NULL,
-                                NULL);
-
-                            helper->password = HeapAlloc(GetProcessHeap(), 0, 
-                                    helper->pwlen);
-
-                            WideCharToMultiByte(CP_UNIXCP, WC_NO_BEST_FIT_CHARS,
-                                auth_data->Password, auth_data->PasswordLength,
-                                helper->password, helper->pwlen, NULL, NULL);
-                        }
+                        ntlm_cred->pwlen = WideCharToMultiByte(CP_UNIXCP, 
+                                                               WC_NO_BEST_FIT_CHARS, auth_data->Password, 
+                                                               auth_data->PasswordLength, NULL, 0, NULL,
+                                                               NULL);
+                        
+                        ntlm_cred->password = HeapAlloc(GetProcessHeap(), 0, 
+                                                        ntlm_cred->pwlen);
+                        
+                        WideCharToMultiByte(CP_UNIXCP, WC_NO_BEST_FIT_CHARS,
+                                            auth_data->Password, auth_data->PasswordLength,
+                                            ntlm_cred->password, ntlm_cred->pwlen, NULL, NULL);
                     }
-
-                    phCredential->dwUpper = fCredentialUse;
-                    phCredential->dwLower = (ULONG_PTR)helper;
-                    TRACE("ACH phCredential->dwUpper: 0x%08lx, dwLower: 0x%08lx\n",
-                            phCredential->dwUpper, phCredential->dwLower);
                 }
+
+                phCredential->dwUpper = fCredentialUse;
+                phCredential->dwLower = (ULONG_PTR)ntlm_cred;
+                TRACE("ACH phCredential->dwUpper: 0x%08lx, dwLower: 0x%08lx\n",
+                      phCredential->dwUpper, phCredential->dwLower);
                 ret = SEC_E_OK;
                 break;
             }
@@ -249,10 +242,7 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
             phCredential = NULL;
             ret = SEC_E_UNKNOWN_CREDENTIALS;
     }
-    
 
-    HeapFree(GetProcessHeap(), 0, client_user_arg);
-    HeapFree(GetProcessHeap(), 0, client_domain_arg);
     HeapFree(GetProcessHeap(), 0, username);
     HeapFree(GetProcessHeap(), 0, domain);
 
@@ -400,7 +390,8 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
  PSecBufferDesc pOutput, ULONG *pfContextAttr, PTimeStamp ptsExpiry)
 {
     SECURITY_STATUS ret;
-    PNegoHelper helper;
+    PNtlmCredentials ntlm_cred = NULL;
+    PNegoHelper helper = NULL;
     ULONG ctxt_attr = 0;
     char* buffer, *want_flags = NULL;
     PBYTE bin;
@@ -439,6 +430,10 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
 
     if((phContext == NULL) && (pInput == NULL))
     {
+        static char helper_protocol[] = "--helper-protocol=ntlmssp-client-1";
+        static CHAR credentials_argv[] = "--use-cached-creds";
+        SEC_CHAR *client_argv[6];
+
         TRACE("First time in ISC()\n");
 
         if(!phCredential)
@@ -447,13 +442,55 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
         /* As the server side of sspi never calls this, make sure that
          * the handler is a client handler.
          */
-        helper = (PNegoHelper)phCredential->dwLower;
-        if(helper->mode != NTLM_CLIENT)
+        ntlm_cred = (PNtlmCredentials)phCredential->dwLower;
+        if(ntlm_cred->mode != NTLM_CLIENT)
         {
-            TRACE("Helper mode = %d\n", helper->mode);
+            TRACE("Cred mode = %d\n", ntlm_cred->mode);
             return SEC_E_INVALID_HANDLE;
         }
 
+        client_argv[0] = ntlm_auth;
+        client_argv[1] = helper_protocol;
+        client_argv[2] = ntlm_cred->username_arg;
+        client_argv[3] = ntlm_cred->domain_arg;
+        client_argv[4] = credentials_argv;
+        client_argv[5] = NULL;
+
+        if((ret = fork_helper(&helper, ntlm_auth, client_argv)) != SEC_E_OK)
+            goto isc_end;
+
+        helper->mode = NTLM_CLIENT;
+        helper->session_key = HeapAlloc(GetProcessHeap(), 0, 16);
+        if (!helper->session_key)
+        {
+            cleanup_helper(helper);
+            ret = SEC_E_INSUFFICIENT_MEMORY;
+            goto isc_end;
+        }
+
+        /* Generate the dummy session key = MD4(MD4(password))*/
+        if(ntlm_cred->password)
+        {
+            SEC_WCHAR *unicode_password;
+            int passwd_lenW;
+
+            TRACE("Converting password to unicode.\n");
+            passwd_lenW = MultiByteToWideChar(CP_ACP, 0,
+                                              (LPCSTR)ntlm_cred->password, ntlm_cred->pwlen,
+                                              NULL, 0);
+            unicode_password = HeapAlloc(GetProcessHeap(), 0,
+                                         passwd_lenW * sizeof(SEC_WCHAR));
+            MultiByteToWideChar(CP_ACP, 0, (LPCSTR)ntlm_cred->password,
+                                ntlm_cred->pwlen, unicode_password, passwd_lenW);
+
+            SECUR32_CreateNTLMv1SessionKey((PBYTE)unicode_password,
+                                           passwd_lenW * sizeof(SEC_WCHAR), helper->session_key);
+
+            HeapFree(GetProcessHeap(), 0, unicode_password);
+        }
+        else
+            memset(helper->session_key, 0, 16);
+
         /* Allocate space for a maximal string of 
          * "SF NTLMSSP_FEATURE_SIGN NTLMSSP_FEATURE_SEAL
          * NTLMSSP_FEATURE_SESSION_KEY"
@@ -461,6 +498,7 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
         want_flags = HeapAlloc(GetProcessHeap(), 0, 73);
         if(want_flags == NULL)
         {
+            cleanup_helper(helper);
             ret = SEC_E_INSUFFICIENT_MEMORY;
             goto isc_end;
         }
@@ -504,11 +542,14 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
 
         /* If no password is given, try to use cached credentials. Fall back to an empty
          * password if this failed. */
-        if(helper->password == NULL)
+        if(ntlm_cred->password == NULL)
         {
             lstrcpynA(buffer, "OK", max_len-1);
             if((ret = run_helper(helper, buffer, max_len, &buffer_len)) != SEC_E_OK)
+            {
+                cleanup_helper(helper);
                 goto isc_end;
+            }
             /* If the helper replied with "PW", using cached credentials failed */
             if(!strncmp(buffer, "PW", 2))
             {
@@ -521,16 +562,22 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
         else
         {
             lstrcpynA(buffer, "PW ", max_len-1);
-            if((ret = encodeBase64((unsigned char*)helper->password,
-                        helper->pwlen, buffer+3,
+            if((ret = encodeBase64((unsigned char*)ntlm_cred->password,
+                        ntlm_cred->pwlen, buffer+3,
                         max_len-3, &buffer_len)) != SEC_E_OK)
+            {
+                cleanup_helper(helper);
                 goto isc_end;
+            }
 
         }
 
         TRACE("Sending to helper: %s\n", debugstr_a(buffer));
         if((ret = run_helper(helper, buffer, max_len, &buffer_len)) != SEC_E_OK)
+        {
+            cleanup_helper(helper);
             goto isc_end;
+        }
 
         TRACE("Helper returned %s\n", debugstr_a(buffer));
 
@@ -548,7 +595,10 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
         lstrcpynA(buffer, "YR", max_len-1);
 
         if((ret = run_helper(helper, buffer, max_len, &buffer_len)) != SEC_E_OK)
+        {
+            cleanup_helper(helper);
             goto isc_end;
+        }
 
         TRACE("%s\n", buffer);
 
@@ -557,11 +607,15 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
             /* Something borked */
             TRACE("Helper returned %c%c\n", buffer[0], buffer[1]);
             ret = SEC_E_INTERNAL_ERROR;
+            cleanup_helper(helper);
             goto isc_end;
         }
         if((ret = decodeBase64(buffer+3, buffer_len-3, bin,
                         max_len-1, &bin_len)) != SEC_E_OK)
+        {
+            cleanup_helper(helper);
             goto isc_end;
+        }
 
         /* put the decoded client blob into the out buffer */
 
@@ -655,6 +709,12 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
     {
         TRACE("no SECBUFFER_TOKEN buffer could be found\n");
         ret = SEC_E_BUFFER_TOO_SMALL;
+        if ((phContext == NULL) && (pInput == NULL))
+        {
+            cleanup_helper(helper);
+            phNewContext->dwUpper = 0;
+            phNewContext->dwLower = 0;
+        }
         goto isc_end;
     }
 
@@ -667,6 +727,12 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
     {
         TRACE("out buffer is NULL or has not enough space\n");
         ret = SEC_E_BUFFER_TOO_SMALL;
+        if ((phContext == NULL) && (pInput == NULL))
+        {
+            cleanup_helper(helper);
+            phNewContext->dwUpper = 0;
+            phNewContext->dwLower = 0;
+        }
         goto isc_end;
     }
 
@@ -674,6 +740,12 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
     {
         TRACE("out buffer is NULL\n");
         ret = SEC_E_INTERNAL_ERROR;
+        if ((phContext == NULL) && (pInput == NULL))
+        {
+            cleanup_helper(helper);
+            phNewContext->dwUpper = 0;
+            phNewContext->dwLower = 0;
+        }
         goto isc_end;
     }
 
@@ -705,33 +777,7 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
             goto isc_end;
 
         if(strncmp(buffer, "BH", 2) == 0)
-        {
             TRACE("No key negotiated.\n");
-            helper->valid_session_key = FALSE;
-            helper->session_key = HeapAlloc(GetProcessHeap(), 0, 16);
-            /*Generate the dummy session key = MD4(MD4(password))*/
-            if(helper->password)
-            {
-                SEC_WCHAR *unicode_password;
-                int passwd_lenW;
-
-                TRACE("Converting password to unicode.\n");
-                passwd_lenW = MultiByteToWideChar(CP_ACP, 0,
-                        (LPCSTR)helper->password, helper->pwlen,
-                        NULL, 0);
-                unicode_password = HeapAlloc(GetProcessHeap(), 0,
-                        passwd_lenW * sizeof(SEC_WCHAR));
-                MultiByteToWideChar(CP_ACP, 0, (LPCSTR)helper->password,
-                        helper->pwlen, unicode_password, passwd_lenW);
-
-                SECUR32_CreateNTLMv1SessionKey((PBYTE)unicode_password,
-                        passwd_lenW * sizeof(SEC_WCHAR), helper->session_key);
-
-                HeapFree(GetProcessHeap(), 0, unicode_password);
-            }
-            else
-                memset(helper->session_key, 0, 16);
-        }
         else if(strncmp(buffer, "GK ", 3) == 0)
         {
             if((ret = decodeBase64(buffer+3, buffer_len-3, bin, max_len, 
@@ -741,6 +787,7 @@ static SECURITY_STATUS SEC_ENTRY ntlm_In
             }
             TRACE("Session key is %s\n", debugstr_a(buffer+3));
             helper->valid_session_key = TRUE;
+            HeapFree(GetProcessHeap(), 0, helper->session_key);
             helper->session_key = HeapAlloc(GetProcessHeap(), 0, bin_len);
             if(!helper->session_key)
             {
@@ -820,31 +867,40 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
     int buffer_len, bin_len, max_len = NTLM_MAX_BUF;
     ULONG ctxt_attr = 0;
     PNegoHelper helper;
+    PNtlmCredentials ntlm_cred;
 
     TRACE("%p %p %p %d %d %p %p %p %p\n", phCredential, phContext, pInput,
      fContextReq, TargetDataRep, phNewContext, pOutput, pfContextAttr,
      ptsExpiry);
 
-    if (!phCredential)
-        return SEC_E_INVALID_HANDLE;
-
-    helper = (PNegoHelper)phCredential->dwLower;
-
     buffer = HeapAlloc(GetProcessHeap(), 0, sizeof(char) * NTLM_MAX_BUF);
     bin    = HeapAlloc(GetProcessHeap(),0, sizeof(BYTE) * NTLM_MAX_BUF);
 
-    if(helper->mode != NTLM_SERVER)
-    {
-        ret = SEC_E_INVALID_HANDLE;
-        goto asc_end;
-    }
-
     if(TargetDataRep == SECURITY_NETWORK_DREP){
         TRACE("Using SECURITY_NETWORK_DREP\n");
     }
 
     if(phContext == NULL)
     {
+        static CHAR server_helper_protocol[] = "--helper-protocol=squid-2.5-ntlmssp";
+        SEC_CHAR *server_argv[] = { ntlm_auth,
+            server_helper_protocol,
+            NULL };
+
+        if (!phCredential)
+        {
+            ret = SEC_E_INVALID_HANDLE;
+            goto asc_end;
+        }
+
+        ntlm_cred = (PNtlmCredentials)phCredential->dwLower;
+
+        if(ntlm_cred->mode != NTLM_SERVER)
+        {
+            ret = SEC_E_INVALID_HANDLE;
+            goto asc_end;
+        }
+        
         /* This is the first call to AcceptSecurityHandle */
         if(pInput == NULL)
         {
@@ -866,12 +922,21 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
         else
             bin_len = pInput->pBuffers[0].cbBuffer;
 
+        if( (ret = fork_helper(&helper, ntlm_auth, server_argv)) !=
+            SEC_E_OK)
+        {
+            ret = SEC_E_INTERNAL_ERROR;
+            goto asc_end;
+        }
+        helper->mode = NTLM_SERVER;
+        
         /* Handle all the flags */
         want_flags = HeapAlloc(GetProcessHeap(), 0, 73);
         if(want_flags == NULL)
         {
             TRACE("Failed to allocate memory for the want_flags!\n");
             ret = SEC_E_INSUFFICIENT_MEMORY;
+            cleanup_helper(helper);
             goto asc_end;
         }
         lstrcpyA(want_flags, "SF");
@@ -921,7 +986,10 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
             lstrcpynA(buffer, want_flags, max_len - 1);
             if((ret = run_helper(helper, buffer, max_len, &buffer_len)) !=
                     SEC_E_OK)
+            {
+                cleanup_helper(helper);
                 goto asc_end;
+            }
             if(!strncmp(buffer, "BH", 2))
                 TRACE("Helper doesn't understand new command set\n");
         }
@@ -935,6 +1003,7 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
         if((ret = encodeBase64(bin, bin_len, buffer+3, max_len-3,
                     &buffer_len)) != SEC_E_OK)
         {
+            cleanup_helper(helper);
             goto asc_end;
         }
 
@@ -943,6 +1012,7 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
         if((ret = run_helper(helper, buffer, max_len, &buffer_len)) !=
                     SEC_E_OK)
         {
+            cleanup_helper(helper);
             goto asc_end;
         }
 
@@ -952,12 +1022,14 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
         if(strncmp(buffer, "TT ", 3) != 0)
         {
             ret = SEC_E_INTERNAL_ERROR;
+            cleanup_helper(helper);
             goto asc_end;
         }
 
         if((ret = decodeBase64(buffer+3, buffer_len-3, bin, max_len,
                         &bin_len)) != SEC_E_OK)
         {
+            cleanup_helper(helper);
             goto asc_end;
         }
 
@@ -965,12 +1037,14 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
         if(pOutput == NULL)
         {
             ret = SEC_E_INSUFFICIENT_MEMORY;
+            cleanup_helper(helper);
             goto asc_end;
         }
 
         if(pOutput->cBuffers < 1)
         {
             ret = SEC_E_INSUFFICIENT_MEMORY;
+            cleanup_helper(helper);
             goto asc_end;
         }
 
@@ -995,6 +1069,20 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Ac
             goto asc_end;
         }
 
+        if(!phContext)
+        {
+            ret = SEC_E_INVALID_HANDLE;
+            goto asc_end;
+        }
+
+        helper = (PNegoHelper)phContext->dwLower;
+
+        if(helper->mode != NTLM_SERVER)
+        {
+            ret = SEC_E_INVALID_HANDLE;
+            goto asc_end;
+        }
+        
         if(pInput->pBuffers[0].cbBuffer > max_len)
         {
             ret = SEC_E_INVALID_TOKEN;
@@ -1146,6 +1234,8 @@ static SECURITY_STATUS SEC_ENTRY ntlm_De
     HeapFree(GetProcessHeap(), 0, helper->crypt.ntlm2.recv_sign_key);
     HeapFree(GetProcessHeap(), 0, helper->crypt.ntlm2.recv_seal_key);
 
+    cleanup_helper(helper);
+
     return SEC_E_OK;
 }
 
@@ -1500,13 +1590,14 @@ static SECURITY_STATUS SEC_ENTRY ntlm_Fr
     SECURITY_STATUS ret;
 
     if(phCredential){
-        PNegoHelper helper = (PNegoHelper) phCredential->dwLower;
+        PNtlmCredentials ntlm_cred = (PNtlmCredentials) phCredential->dwLower;
         phCredential->dwUpper = 0;
         phCredential->dwLower = 0;
-        if (helper->password)
-            memset(helper->password, 0, helper->pwlen);
-        HeapFree(GetProcessHeap(), 0, helper->password);
-        cleanup_helper(helper);
+        if (ntlm_cred->password)
+            memset(ntlm_cred->password, 0, ntlm_cred->pwlen);
+        HeapFree(GetProcessHeap(), 0, ntlm_cred->password);
+        HeapFree(GetProcessHeap(), 0, ntlm_cred->username_arg);
+        HeapFree(GetProcessHeap(), 0, ntlm_cred->domain_arg);
         ret = SEC_E_OK;
     }
     else
diff --git a/dlls/secur32/secur32_priv.h b/dlls/secur32/secur32_priv.h
index 8e80dea..a11cc20 100644
--- a/dlls/secur32/secur32_priv.h
+++ b/dlls/secur32/secur32_priv.h
@@ -61,8 +61,6 @@ typedef struct tag_arc4_info {
 typedef struct _NegoHelper {
     pid_t helper_pid;
     HelperMode mode;
-    SEC_CHAR *password;
-    int pwlen;
     int pipe_in;
     int pipe_out;
     int major;


More information about the wine-patches mailing list