From: Ondrej Holy Date: Mon, 20 Aug 2018 08:55:29 +0000 (+0200) Subject: winpr/utils/ntlm: Prevent releasing function argument X-Git-Tag: 2.0.0-rc4~110^2~4 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4813a70897d21da3e3c207e5bcc698b8e8369ab2;p=platform%2Fupstream%2Ffreerdp.git winpr/utils/ntlm: Prevent releasing function argument The patch changes API of functions instead of fixing unused and broken code. freed_arg: "free" frees parameter "NtHash". --- diff --git a/winpr/include/winpr/ntlm.h b/winpr/include/winpr/ntlm.h index 9649f53..c5db0a9 100644 --- a/winpr/include/winpr/ntlm.h +++ b/winpr/include/winpr/ntlm.h @@ -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 } diff --git a/winpr/libwinpr/utils/ntlm.c b/winpr/libwinpr/utils/ntlm.c index b88285a..bdfb696 100644 --- a/winpr/libwinpr/utils/ntlm.c +++ b/winpr/libwinpr/utils/ntlm.c @@ -32,40 +32,38 @@ * 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; }