From 93c5058aa565e06336aa381533e2f8c3cf7914ed Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 16 May 2023 15:12:57 +0200 Subject: [PATCH] [winpr,crypto] rc4 allocation check * check success of winpr_RC4_New * WINPR_ASSERT context when used --- winpr/libwinpr/crypto/cipher.c | 6 +++++- winpr/libwinpr/sspi/NTLM/ntlm.c | 15 ++++++++++++++- winpr/libwinpr/sspi/NTLM/ntlm.h | 2 +- winpr/libwinpr/sspi/NTLM/ntlm_compute.c | 13 ++++++++++++- winpr/libwinpr/sspi/NTLM/ntlm_compute.h | 2 +- winpr/libwinpr/sspi/NTLM/ntlm_message.c | 6 ++++-- winpr/libwinpr/sspi/Negotiate/negotiate.c | 5 ++++- 7 files changed, 41 insertions(+), 8 deletions(-) diff --git a/winpr/libwinpr/crypto/cipher.c b/winpr/libwinpr/crypto/cipher.c index 6d18e7a99..7336bb9c3 100644 --- a/winpr/libwinpr/crypto/cipher.c +++ b/winpr/libwinpr/crypto/cipher.c @@ -19,6 +19,7 @@ #include #include +#include #include @@ -115,7 +116,9 @@ BOOL winpr_RC4_Update(WINPR_RC4_CTX* ctx, size_t length, const BYTE* input, BYTE if (length > INT_MAX) return FALSE; - EVP_CipherUpdate((EVP_CIPHER_CTX*)ctx, output, &outputLength, input, (int)length); + WINPR_ASSERT(ctx); + if (EVP_CipherUpdate((EVP_CIPHER_CTX*)ctx, output, &outputLength, input, (int)length) != 1) + return FALSE; return TRUE; #elif defined(WITH_MBEDTLS) && defined(MBEDTLS_ARC4_C) @@ -641,6 +644,7 @@ BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const BYTE* input, size_t ilen, return FALSE; } + WINPR_ASSERT(ctx); if (EVP_CipherUpdate((EVP_CIPHER_CTX*)ctx, output, &outl, input, (int)ilen) == 1) { *olen = (size_t)outl; diff --git a/winpr/libwinpr/sspi/NTLM/ntlm.c b/winpr/libwinpr/sspi/NTLM/ntlm.c index e8cfef96b..41e7fedd2 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm.c @@ -1458,7 +1458,7 @@ NTLM_STATE ntlm_get_state(NTLM_CONTEXT* ntlm) return ntlm->state; } -void ntlm_reset_cipher_state(PSecHandle phContext) +BOOL ntlm_reset_cipher_state(PSecHandle phContext) { NTLM_CONTEXT* context = sspi_SecureHandleGetLowerPointer(phContext); @@ -1468,5 +1468,18 @@ void ntlm_reset_cipher_state(PSecHandle phContext) winpr_RC4_Free(context->RecvRc4Seal); context->SendRc4Seal = winpr_RC4_New(context->RecvSealingKey, 16); context->RecvRc4Seal = winpr_RC4_New(context->SendSealingKey, 16); + + if (!context->SendRc4Seal) + { + WLog_ERR(TAG, "Failed to allocate context->SendRc4Seal"); + return FALSE; + } + if (!context->RecvRc4Seal) + { + WLog_ERR(TAG, "Failed to allocate context->RecvRc4Seal"); + return FALSE; + } } + + return TRUE; } diff --git a/winpr/libwinpr/sspi/NTLM/ntlm.h b/winpr/libwinpr/sspi/NTLM/ntlm.h index 1622ab8bc..bbc512364 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm.h +++ b/winpr/libwinpr/sspi/NTLM/ntlm.h @@ -287,7 +287,7 @@ const char* ntlm_message_type_string(UINT32 messageType); const char* ntlm_state_string(NTLM_STATE state); void ntlm_change_state(NTLM_CONTEXT* ntlm, NTLM_STATE state); NTLM_STATE ntlm_get_state(NTLM_CONTEXT* ntlm); -void ntlm_reset_cipher_state(PSecHandle phContext); +BOOL ntlm_reset_cipher_state(PSecHandle phContext); SECURITY_STATUS ntlm_computeProofValue(NTLM_CONTEXT* ntlm, SecBuffer* ntproof); SECURITY_STATUS ntlm_computeMicValue(NTLM_CONTEXT* ntlm, SecBuffer* micvalue); diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_compute.c b/winpr/libwinpr/sspi/NTLM/ntlm_compute.c index 29bb00050..b91b84cec 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm_compute.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm_compute.c @@ -788,7 +788,7 @@ BOOL ntlm_generate_server_sealing_key(NTLM_CONTEXT* context) * @param context A pointer to the NTLM context */ -void ntlm_init_rc4_seal_states(NTLM_CONTEXT* context) +BOOL ntlm_init_rc4_seal_states(NTLM_CONTEXT* context) { WINPR_ASSERT(context); if (context->server) @@ -813,6 +813,17 @@ void ntlm_init_rc4_seal_states(NTLM_CONTEXT* context) context->RecvRc4Seal = winpr_RC4_New(context->ServerSealingKey, sizeof(context->ServerSealingKey)); } + if (!context->SendRc4Seal) + { + WLog_ERR(TAG, "Failed to allocate context->SendRc4Seal"); + return FALSE; + } + if (!context->RecvRc4Seal) + { + WLog_ERR(TAG, "Failed to allocate context->RecvRc4Seal"); + return FALSE; + } + return TRUE; } BOOL ntlm_compute_message_integrity_check(NTLM_CONTEXT* context, BYTE* mic, UINT32 size) diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_compute.h b/winpr/libwinpr/sspi/NTLM/ntlm_compute.h index a5572b504..006a449ed 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm_compute.h +++ b/winpr/libwinpr/sspi/NTLM/ntlm_compute.h @@ -57,7 +57,7 @@ BOOL ntlm_generate_client_signing_key(NTLM_CONTEXT* context); BOOL ntlm_generate_server_signing_key(NTLM_CONTEXT* context); BOOL ntlm_generate_client_sealing_key(NTLM_CONTEXT* context); BOOL ntlm_generate_server_sealing_key(NTLM_CONTEXT* context); -void ntlm_init_rc4_seal_states(NTLM_CONTEXT* context); +BOOL ntlm_init_rc4_seal_states(NTLM_CONTEXT* context); BOOL ntlm_compute_message_integrity_check(NTLM_CONTEXT* context, BYTE* mic, UINT32 size); diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_message.c b/winpr/libwinpr/sspi/NTLM/ntlm_message.c index c095bdcff..21f08dac9 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm_message.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm_message.c @@ -789,7 +789,8 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf if (!ntlm_generate_server_sealing_key(context)) goto fail; /* Initialize RC4 seal state using client sealing key */ - ntlm_init_rc4_seal_states(context); + if (!ntlm_init_rc4_seal_states(context)) + goto fail; #if defined(WITH_DEBUG_NTLM) ntlm_print_authentication_complete(context); #endif @@ -1172,7 +1173,8 @@ SECURITY_STATUS ntlm_read_AuthenticateMessage(NTLM_CONTEXT* context, PSecBuffer if (!ntlm_generate_server_sealing_key(context)) return SEC_E_INTERNAL_ERROR; /* Initialize RC4 seal state */ - ntlm_init_rc4_seal_states(context); + if (!ntlm_init_rc4_seal_states(context)) + return SEC_E_INTERNAL_ERROR; #if defined(WITH_DEBUG_NTLM) ntlm_print_authentication_complete(context); #endif diff --git a/winpr/libwinpr/sspi/Negotiate/negotiate.c b/winpr/libwinpr/sspi/Negotiate/negotiate.c index 453e26c33..505d7d28a 100644 --- a/winpr/libwinpr/sspi/Negotiate/negotiate.c +++ b/winpr/libwinpr/sspi/Negotiate/negotiate.c @@ -596,7 +596,10 @@ static SECURITY_STATUS negotiate_mic_exchange(NEGOTIATE_CONTEXT* context, NegTok /* When using NTLM cipher states need to be reset after mic exchange */ if (_tcscmp(sspi_SecureHandleGetUpperPointer(&context->sub_context), NTLM_SSP_NAME) == 0) - ntlm_reset_cipher_state(&context->sub_context); + { + if (!ntlm_reset_cipher_state(&context->sub_context)) + return SEC_E_INTERNAL_ERROR; + } return SEC_E_OK; }