Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fixed oob read in ntlm_read_ntlm_v2_response
  • Loading branch information
akallabeth committed May 5, 2020
1 parent f59ad0f commit c098f21
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions winpr/libwinpr/sspi/NTLM/ntlm_compute.c
Expand Up @@ -124,6 +124,9 @@ void ntlm_print_version_info(NTLM_VERSION_INFO* versionInfo)
static int ntlm_read_ntlm_v2_client_challenge(wStream* s, NTLMv2_CLIENT_CHALLENGE* challenge)
{
size_t size;
if (Stream_GetRemainingLength(s) < 28)
return -1;
Stream_Read_UINT8(s, challenge->RespType);
Stream_Read_UINT8(s, challenge->HiRespType);
Stream_Read_UINT16(s, challenge->Reserved1);
Expand Down Expand Up @@ -163,6 +166,8 @@ static int ntlm_write_ntlm_v2_client_challenge(wStream* s, NTLMv2_CLIENT_CHALLEN
int ntlm_read_ntlm_v2_response(wStream* s, NTLMv2_RESPONSE* response)
{
if (Stream_GetRemainingLength(s) < 16)
return -1;
Stream_Read(s, response->Response, 16);
return ntlm_read_ntlm_v2_client_challenge(s, &(response->Challenge));
}
Expand Down

3 comments on commit c098f21

@tcullum-rh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akallabeth In ntlm_read_ntlm_v2_client_challenge(), why if (Stream_GetRemainingLength(s) < 28) ? The code looks like it actually reads 112 bytes from the stream. Am I missing something?

@akallabeth
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tcullum-rh No, the check is correct. 28 bytes are read in and the remaining length copied to NTLM_AV_PAIR

@tcullum-rh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akallabeth ah I misunderstood the calls as "8 UINTs" instead of "1 byte". Thanks for clarifying, your response, and all the hard work!

Please sign in to comment.