winpr/utils/ntlm: Prevent releasing function argument
authorOndrej Holy <oholy@redhat.com>
Mon, 20 Aug 2018 08:55:29 +0000 (10:55 +0200)
committerOndrej Holy <oholy@redhat.com>
Thu, 23 Aug 2018 07:11:24 +0000 (09:11 +0200)
The patch changes API of functions instead of fixing unused and broken code.

freed_arg: "free" frees parameter "NtHash".

winpr/include/winpr/ntlm.h
winpr/libwinpr/utils/ntlm.c

index 9649f53..c5db0a9 100644 (file)
@@ -36,18 +36,18 @@ typedef SECURITY_STATUS(*psPeerComputeNtlmHash)(void* client,
         const SecBuffer* ntproofvalue, const BYTE* randkey, const BYTE* mic, const SecBuffer* micvalue,
         BYTE* ntlmhash);
 
-WINPR_API BYTE* NTOWFv1W(LPWSTR Password, UINT32 PasswordLength, BYTE* NtHash);
-WINPR_API BYTE* NTOWFv1A(LPSTR Password, UINT32 PasswordLength, BYTE* NtHash);
+WINPR_API BOOL NTOWFv1W(LPWSTR Password, UINT32 PasswordLength, BYTE* NtHash);
+WINPR_API BOOL NTOWFv1A(LPSTR Password, UINT32 PasswordLength, BYTE* NtHash);
 
-WINPR_API BYTE* NTOWFv2W(LPWSTR Password, UINT32 PasswordLength, LPWSTR User,
-                         UINT32 UserLength, LPWSTR Domain, UINT32 DomainLength, BYTE* NtHash);
-WINPR_API BYTE* NTOWFv2A(LPSTR Password, UINT32 PasswordLength, LPSTR User,
-                         UINT32 UserLength, LPSTR Domain, UINT32 DomainLength, BYTE* NtHash);
+WINPR_API BOOL NTOWFv2W(LPWSTR Password, UINT32 PasswordLength, LPWSTR User,
+                        UINT32 UserLength, LPWSTR Domain, UINT32 DomainLength, BYTE* NtHash);
+WINPR_API BOOL NTOWFv2A(LPSTR Password, UINT32 PasswordLength, LPSTR User,
+                        UINT32 UserLength, LPSTR Domain, UINT32 DomainLength, BYTE* NtHash);
 
-WINPR_API BYTE* NTOWFv2FromHashW(BYTE* NtHashV1, LPWSTR User, UINT32 UserLength,
-                                 LPWSTR Domain, UINT32 DomainLength, BYTE* NtHash);
-WINPR_API BYTE* NTOWFv2FromHashA(BYTE* NtHashV1, LPSTR User, UINT32 UserLength,
-                                 LPSTR Domain, UINT32 DomainLength, BYTE* NtHash);
+WINPR_API BOOL NTOWFv2FromHashW(BYTE* NtHashV1, LPWSTR User, UINT32 UserLength,
+                                LPWSTR Domain, UINT32 DomainLength, BYTE* NtHash);
+WINPR_API BOOL NTOWFv2FromHashA(BYTE* NtHashV1, LPSTR User, UINT32 UserLength,
+                                LPSTR Domain, UINT32 DomainLength, BYTE* NtHash);
 
 #ifdef __cplusplus
 }
index b88285a..bdfb696 100644 (file)
  * EndDefine
  */
 
-BYTE* NTOWFv1W(LPWSTR Password, UINT32 PasswordLength, BYTE* NtHash)
+BOOL NTOWFv1W(LPWSTR Password, UINT32 PasswordLength, BYTE* NtHash)
 {
-       BOOL allocate = !NtHash;
-
-       if (!Password)
-               return NULL;
-
-       if (!NtHash && !(NtHash = malloc(WINPR_MD4_DIGEST_LENGTH)))
-               return NULL;
+       if (!Password || !NtHash)
+               return FALSE;
 
        if (!winpr_Digest(WINPR_MD_MD4, (BYTE*) Password, (size_t) PasswordLength, NtHash,
                          WINPR_MD4_DIGEST_LENGTH))
-       {
-               if (allocate)
-               {
-                       free(NtHash);
-                       NtHash = NULL;
-               }
-       }
+               return FALSE;
 
-       return NtHash;
+       return TRUE;
 }
 
-BYTE* NTOWFv1A(LPSTR Password, UINT32 PasswordLength, BYTE* NtHash)
+BOOL NTOWFv1A(LPSTR Password, UINT32 PasswordLength, BYTE* NtHash)
 {
        LPWSTR PasswordW = NULL;
+       BOOL result = FALSE;
+
+       if (!NtHash)
+               return FALSE;
 
        if (!(PasswordW = (LPWSTR) calloc(PasswordLength, 2)))
-               return NULL;
+               return FALSE;
 
        MultiByteToWideChar(CP_ACP, 0, Password, PasswordLength, PasswordW, PasswordLength);
-       NtHash = NTOWFv1W(PasswordW, PasswordLength * 2, NtHash);
+
+       if (!NTOWFv1W(PasswordW, PasswordLength * 2, NtHash))
+               goto out_fail;
+
+       result = TRUE;
+out_fail:
        free(PasswordW);
-       return NtHash;
+       return result;
 }
 
 /**
@@ -75,30 +73,21 @@ BYTE* NTOWFv1A(LPSTR Password, UINT32 PasswordLength, BYTE* NtHash)
  * EndDefine
  */
 
-BYTE* NTOWFv2W(LPWSTR Password, UINT32 PasswordLength, LPWSTR User,
-               UINT32 UserLength, LPWSTR Domain, UINT32 DomainLength, BYTE* NtHash)
+BOOL NTOWFv2W(LPWSTR Password, UINT32 PasswordLength, LPWSTR User,
+              UINT32 UserLength, LPWSTR Domain, UINT32 DomainLength, BYTE* NtHash)
 {
        BYTE* buffer;
        BYTE NtHashV1[16];
-       BYTE* result = NtHash;
-
-       if ((!User) || (!Password))
-               return NULL;
+       BOOL result = FALSE;
 
-       if (!NtHash && !(NtHash = (BYTE*) malloc(WINPR_MD4_DIGEST_LENGTH)))
-               return NULL;
+       if ((!User) || (!Password) || (!NtHash))
+               return FALSE;
 
        if (!NTOWFv1W(Password, PasswordLength, NtHashV1))
-       {
-               free(NtHash);
-               return NULL;
-       }
+               return FALSE;
 
        if (!(buffer = (BYTE*) malloc(UserLength + DomainLength)))
-       {
-               free(NtHash);
-               return NULL;
-       }
+               return FALSE;
 
        /* Concatenate(UpperCase(User), Domain) */
        CopyMemory(buffer, User, UserLength);
@@ -108,18 +97,25 @@ BYTE* NTOWFv2W(LPWSTR Password, UINT32 PasswordLength, LPWSTR User,
        /* Compute the HMAC-MD5 hash of the above value using the NTLMv1 hash as the key, the result is the NTLMv2 hash */
        if (!winpr_HMAC(WINPR_MD_MD5, NtHashV1, 16, buffer, UserLength + DomainLength, NtHash,
                        WINPR_MD4_DIGEST_LENGTH))
-               result = NULL;
+               goto out_fail;
 
+       result = TRUE;
+out_fail:
        free(buffer);
        return result;
 }
 
-BYTE* NTOWFv2A(LPSTR Password, UINT32 PasswordLength, LPSTR User,
-               UINT32 UserLength, LPSTR Domain, UINT32 DomainLength, BYTE* NtHash)
+BOOL NTOWFv2A(LPSTR Password, UINT32 PasswordLength, LPSTR User,
+              UINT32 UserLength, LPSTR Domain, UINT32 DomainLength, BYTE* NtHash)
 {
        LPWSTR UserW = NULL;
        LPWSTR DomainW = NULL;
        LPWSTR PasswordW = NULL;
+       BOOL result = FALSE;
+
+       if (!NtHash)
+               return FALSE;
+
        UserW = (LPWSTR) calloc(UserLength, 2);
        DomainW = (LPWSTR) calloc(DomainLength, 2);
        PasswordW = (LPWSTR) calloc(PasswordLength, 2);
@@ -130,32 +126,30 @@ BYTE* NTOWFv2A(LPSTR Password, UINT32 PasswordLength, LPSTR User,
        MultiByteToWideChar(CP_ACP, 0, User, UserLength, UserW, UserLength);
        MultiByteToWideChar(CP_ACP, 0, Domain, DomainLength, DomainW, DomainLength);
        MultiByteToWideChar(CP_ACP, 0, Password, PasswordLength, PasswordW, PasswordLength);
-       NtHash = NTOWFv2W(PasswordW, PasswordLength * 2, UserW, UserLength * 2, DomainW, DomainLength * 2,
-                         NtHash);
+
+       if (!NTOWFv2W(PasswordW, PasswordLength * 2, UserW, UserLength * 2, DomainW, DomainLength * 2,
+                     NtHash))
+               goto out_fail;
+
+       result = TRUE;
 out_fail:
        free(UserW);
        free(DomainW);
        free(PasswordW);
-       return NtHash;
+       return result;
 }
 
-BYTE* NTOWFv2FromHashW(BYTE* NtHashV1, LPWSTR User, UINT32 UserLength, LPWSTR Domain,
-                       UINT32 DomainLength, BYTE* NtHash)
+BOOL NTOWFv2FromHashW(BYTE* NtHashV1, LPWSTR User, UINT32 UserLength, LPWSTR Domain,
+                      UINT32 DomainLength, BYTE* NtHash)
 {
        BYTE* buffer;
-       BYTE* result = NtHash;
-
-       if (!User)
-               return NULL;
+       BYTE result = FALSE;
 
-       if (!NtHash && !(NtHash = (BYTE*) malloc(WINPR_MD4_DIGEST_LENGTH)))
-               return NULL;
+       if (!User || !NtHash)
+               return FALSE;
 
        if (!(buffer = (BYTE*) malloc(UserLength + DomainLength)))
-       {
-               free(NtHash);
-               return NULL;
-       }
+               return FALSE;
 
        /* Concatenate(UpperCase(User), Domain) */
        CopyMemory(buffer, User, UserLength);
@@ -169,17 +163,24 @@ BYTE* NTOWFv2FromHashW(BYTE* NtHashV1, LPWSTR User, UINT32 UserLength, LPWSTR Do
        /* Compute the HMAC-MD5 hash of the above value using the NTLMv1 hash as the key, the result is the NTLMv2 hash */
        if (!winpr_HMAC(WINPR_MD_MD5, NtHashV1, 16, buffer, UserLength + DomainLength, NtHash,
                        WINPR_MD4_DIGEST_LENGTH))
-               result = NULL;
+               goto out_fail;
 
+       result = TRUE;
+out_fail:
        free(buffer);
        return result;
 }
 
-BYTE* NTOWFv2FromHashA(BYTE* NtHashV1, LPSTR User, UINT32 UserLength, LPSTR Domain,
-                       UINT32 DomainLength, BYTE* NtHash)
+BOOL NTOWFv2FromHashA(BYTE* NtHashV1, LPSTR User, UINT32 UserLength, LPSTR Domain,
+                      UINT32 DomainLength, BYTE* NtHash)
 {
        LPWSTR UserW = NULL;
        LPWSTR DomainW = NULL;
+       BOOL result = FALSE;
+
+       if (!NtHash)
+               return FALSE;
+
        UserW = (LPWSTR) calloc(UserLength, 2);
        DomainW = (LPWSTR) calloc(DomainLength, 2);
 
@@ -188,9 +189,13 @@ BYTE* NTOWFv2FromHashA(BYTE* NtHashV1, LPSTR User, UINT32 UserLength, LPSTR Doma
 
        MultiByteToWideChar(CP_ACP, 0, User, UserLength, UserW, UserLength);
        MultiByteToWideChar(CP_ACP, 0, Domain, DomainLength, DomainW, DomainLength);
-       NtHash = NTOWFv2FromHashW(NtHashV1, UserW, UserLength * 2, DomainW, DomainLength * 2, NtHash);
+
+       if (!NTOWFv2FromHashW(NtHashV1, UserW, UserLength * 2, DomainW, DomainLength * 2, NtHash))
+               goto out_fail;
+
+       result = TRUE;
 out_fail:
        free(UserW);
        free(DomainW);
-       return NtHash;
+       return result;
 }