CVE-2020-11087: Out-of-bounds Read (CWE-125)
value: In FreeRDP less than or equal to 2.0.0, there is an out-of-bound read in ntlm_read_AuthenticateMessage. This has been fixed in 2.1.0.
CWE description: The product reads data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.4
Published: 2020-05-29T20:15Z
Repository: https://github.com/FreeRDP/FreeRDP
Language: C
Files changed: winpr/libwinpr/sspi/NTLM/ntlm_message.c
Commit date: 2020-05-06 13:31:57 +0200
Commit message: Fixed oob read in ntlm_read_AuthenticateMessage

Diff: diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_message.c b/winpr/libwinpr/sspi/NTLM/ntlm_message.c
index c4ba1d9749..5fc6ec73ba 100644
--- a/winpr/libwinpr/sspi/NTLM/ntlm_message.c
+++ b/winpr/libwinpr/sspi/NTLM/ntlm_message.c
@@ -679,151 +679,106 @@ SECURITY_STATUS ntlm_write_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer bu
 	return SEC_I_CONTINUE_NEEDED;
 }
 
 SECURITY_STATUS ntlm_read_AuthenticateMessage(NTLM_CONTEXT* context, PSecBuffer buffer)
 {
+	SECURITY_STATUS status = SEC_E_INVALID_TOKEN;
 	wStream* s;
 	size_t length;
-	UINT32 flags;
-	NTLM_AV_PAIR* AvFlags;
+	UINT32 flags = 0;
+	NTLM_AV_PAIR* AvFlags = NULL;
 	UINT32 PayloadBufferOffset;
 	NTLM_AUTHENTICATE_MESSAGE* message;
 	SSPI_CREDENTIALS* credentials = context->credentials;
-	flags = 0;
-	AvFlags = NULL;
+
 	message = &context->AUTHENTICATE_MESSAGE;
 	ZeroMemory(message, sizeof(NTLM_AUTHENTICATE_MESSAGE));
 	s = Stream_New((BYTE*)buffer->pvBuffer, buffer->cbBuffer);
 
 	if (!s)
 		return SEC_E_INTERNAL_ERROR;
 
 	if (ntlm_read_message_header(s, (NTLM_MESSAGE_HEADER*)message) < 0)
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (message->MessageType != MESSAGE_TYPE_AUTHENTICATE)
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields(s, &(message->LmChallengeResponse)) <
 	    0) /* LmChallengeResponseFields (8 bytes) */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields(s, &(message->NtChallengeResponse)) <
 	    0) /* NtChallengeResponseFields (8 bytes) */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields(s, &(message->DomainName)) < 0) /* DomainNameFields (8 bytes) */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields(s, &(message->UserName)) < 0) /* UserNameFields (8 bytes) */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields(s, &(message->Workstation)) < 0) /* WorkstationFields (8 bytes) */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields(s, &(message->EncryptedRandomSessionKey)) <
 	    0) /* EncryptedRandomSessionKeyFields (8 bytes) */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
+	if (Stream_GetRemainingLength(s) < 4)
+		goto fail;
 	Stream_Read_UINT32(s, message->NegotiateFlags); /* NegotiateFlags (4 bytes) */
 	context->NegotiateKeyExchange =
 	    (message->NegotiateFlags & NTLMSSP_NEGOTIATE_KEY_EXCH) ? TRUE : FALSE;
 
 	if ((context->NegotiateKeyExchange && !message->EncryptedRandomSessionKey.Len) ||
 	    (!context->NegotiateKeyExchange && message->EncryptedRandomSessionKey.Len))
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INVALID_TOKEN;
-	}
+		goto fail;
 
 	if (message->NegotiateFlags & NTLMSSP_NEGOTIATE_VERSION)
 	{
 		if (ntlm_read_version_info(s, &(message->Version)) < 0) /* Version (8 bytes) */
-		{
-			Stream_Free(s, FALSE);
-			return SEC_E_INVALID_TOKEN;
-		}
+			goto fail;
 	}
 
 	PayloadBufferOffset = Stream_GetPosition(s);
 
+	status = SEC_E_INTERNAL_ERROR;
 	if (ntlm_read_message_fields_buffer(s, &(message->DomainName)) < 0) /* DomainName */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields_buffer(s, &(message->UserName)) < 0) /* UserName */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields_buffer(s, &(message->Workstation)) < 0) /* Workstation */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields_buffer(s, &(message->LmChallengeResponse)) <
 	    0) /* LmChallengeResponse */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	if (ntlm_read_message_fields_buffer(s, &(message->NtChallengeResponse)) <
 	    0) /* NtChallengeResponse */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	if (message->NtChallengeResponse.Len > 0)
 	{
+		int rc;
 		size_t cbAvFlags;
 		wStream* snt =
 		    Stream_New(message->NtChallengeResponse.Buffer, message->NtChallengeResponse.Len);
 
 		if (!snt)
-		{
-			Stream_Free(s, FALSE);
-			return SEC_E_INTERNAL_ERROR;
-		}
-
-		if (ntlm_read_ntlm_v2_response(snt, &(context->NTLMv2Response)) < 0)
-		{
-			Stream_Free(s, FALSE);
-			Stream_Free(snt, FALSE);
-			return SEC_E_INVALID_TOKEN;
-		}
+			goto fail;
 
+		status = SEC_E_INVALID_TOKEN;
+		rc = ntlm_read_ntlm_v2_response(snt, &(context->NTLMv2Response));
 		Stream_Free(snt, FALSE);
+		if (rc < 0)
+			goto fail;
+		status = SEC_E_INTERNAL_ERROR;
+
 		context->NtChallengeResponse.pvBuffer = message->NtChallengeResponse.Buffer;
 		context->NtChallengeResponse.cbBuffer = message->NtChallengeResponse.Len;
 		sspi_SecBufferFree(&(context->ChallengeTargetInfo));
 		context->ChallengeTargetInfo.pvBuffer = (void*)context->NTLMv2Response.Challenge.AvPairs;
 		context->ChallengeTargetInfo.cbBuffer = message->NtChallengeResponse.Len - (28 + 16);
@@ -836,52 +791,43 @@ SECURITY_STATUS ntlm_read_AuthenticateMessage(NTLM_CONTEXT* context, PSecBuffer
 			Data_Read_UINT32(ntlm_av_pair_get_value_pointer(AvFlags), flags);
 	}
 
 	if (ntlm_read_message_fields_buffer(s, &(message->EncryptedRandomSessionKey)) <
 	    0) /* EncryptedRandomSessionKey */
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	if (message->EncryptedRandomSessionKey.Len > 0)
 	{
 		if (message->EncryptedRandomSessionKey.Len != 16)
-		{
-			Stream_Free(s, FALSE);
-			return SEC_E_INVALID_TOKEN;
-		}
+			goto fail;
 
 		CopyMemory(context->EncryptedRandomSessionKey, message->EncryptedRandomSessionKey.Buffer,
 		           16);
 	}
 
 	length = Stream_GetPosition(s);
 
 	if (!sspi_SecBufferAlloc(&context->AuthenticateMessage, length))
-	{
-		Stream_Free(s, FALSE);
-		return SEC_E_INTERNAL_ERROR;
-	}
+		goto fail;
 
 	CopyMemory(context->AuthenticateMessage.pvBuffer, Stream_Buffer(s), length);
 	buffer->cbBuffer = length;
 	Stream_SetPosition(s, PayloadBufferOffset);
 
 	if (flags & MSV_AV_FLAGS_MESSAGE_INTEGRITY_CHECK)
 	{
 		context->MessageIntegrityCheckOffset = (UINT32)Stream_GetPosition(s);
 
+		status = SEC_E_INVALID_TOKEN;
 		if (Stream_GetRemainingLength(s) < 16)
-		{
-			Stream_Free(s, FALSE);
-			return SEC_E_INVALID_TOKEN;
-		}
+			goto fail;
 
 		Stream_Read(s, message->MessageIntegrityCheck, 16);
 	}
 
+	status = SEC_E_INTERNAL_ERROR;
+
 #ifdef WITH_DEBUG_NTLM
 	WLog_DBG(TAG, "AUTHENTICATE_MESSAGE (length = %" PRIu32 ")",
 	         context->AuthenticateMessage.cbBuffer);
 	winpr_HexDump(TAG, WLOG_DEBUG, context->AuthenticateMessage.pvBuffer,
 	              context->AuthenticateMessage.cbBuffer);
@@ -909,38 +855,36 @@ SECURITY_STATUS ntlm_read_AuthenticateMessage(NTLM_CONTEXT* context, PSecBuffer
 	if (message->UserName.Len > 0)
 	{
 		credentials->identity.User = (UINT16*)malloc(message->UserName.Len);
 
 		if (!credentials->identity.User)
-		{
-			Stream_Free(s, FALSE);
-			return SEC_E_INTERNAL_ERROR;
-		}
+			goto fail;
 
 		CopyMemory(credentials->identity.User, message->UserName.Buffer, message->UserName.Len);
 		credentials->identity.UserLength = message->UserName.Len / 2;
 	}
 
 	if (message->DomainName.Len > 0)
 	{
 		credentials->identity.Domain = (UINT16*)malloc(message->DomainName.Len);
 
 		if (!credentials->identity.Domain)
-		{
-			Stream_Free(s, FALSE);
-			return SEC_E_INTERNAL_ERROR;
-		}
+			goto fail;
 
 		CopyMemory(credentials->identity.Domain, message->DomainName.Buffer,
 		           message->DomainName.Len);
 		credentials->identity.DomainLength = message->DomainName.Len / 2;
 	}
 
 	Stream_Free(s, FALSE);
 	/* Computations beyond this point require the NTLM hash of the password */
 	context->state = NTLM_STATE_COMPLETION;
 	return SEC_I_COMPLETE_NEEDED;
+
+fail:
+	Stream_Free(s, FALSE);
+	return status;
 }
 
 /**
  * Send NTLMSSP AUTHENTICATE_MESSAGE.\n
  * AUTHENTICATE_MESSAGE @msdn{cc236643}

Vulnerable code: 	UINT32 flags;
	NTLM_AV_PAIR* AvFlags;
	flags = 0;
	AvFlags = NULL;
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INVALID_TOKEN;
	}
		{
			Stream_Free(s, FALSE);
			return SEC_E_INVALID_TOKEN;
		}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
		{
			Stream_Free(s, FALSE);
			return SEC_E_INTERNAL_ERROR;
		}

		if (ntlm_read_ntlm_v2_response(snt, &(context->NTLMv2Response)) < 0)
		{
			Stream_Free(s, FALSE);
			Stream_Free(snt, FALSE);
			return SEC_E_INVALID_TOKEN;
		}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
		{
			Stream_Free(s, FALSE);
			return SEC_E_INVALID_TOKEN;
		}
	{
		Stream_Free(s, FALSE);
		return SEC_E_INTERNAL_ERROR;
	}
		{
			Stream_Free(s, FALSE);
			return SEC_E_INVALID_TOKEN;
		}
		{
			Stream_Free(s, FALSE);
			return SEC_E_INTERNAL_ERROR;
		}
		{
			Stream_Free(s, FALSE);
			return SEC_E_INTERNAL_ERROR;
		}
Fixed code: 	SECURITY_STATUS status = SEC_E_INVALID_TOKEN;
	UINT32 flags = 0;
	NTLM_AV_PAIR* AvFlags = NULL;

		goto fail;
		goto fail;
		goto fail;
		goto fail;
		goto fail;
		goto fail;
		goto fail;
		goto fail;
	if (Stream_GetRemainingLength(s) < 4)
		goto fail;
		goto fail;
			goto fail;
	status = SEC_E_INTERNAL_ERROR;
		goto fail;
		goto fail;
		goto fail;
		goto fail;
		goto fail;
		int rc;
			goto fail;
		status = SEC_E_INVALID_TOKEN;
		rc = ntlm_read_ntlm_v2_response(snt, &(context->NTLMv2Response));
		if (rc < 0)
			goto fail;
		status = SEC_E_INTERNAL_ERROR;

		goto fail;
			goto fail;
		goto fail;
		status = SEC_E_INVALID_TOKEN;
			goto fail;
	status = SEC_E_INTERNAL_ERROR;

			goto fail;
			goto fail;

fail:
	Stream_Free(s, FALSE);
	return status;