Skip to content

Commit 48361c4

Browse files
committed
Fixed GHSL-2020-100: oob read in ntlm_read_ChallengeMessage
* Added length checks for data read from stream * Unified function resource cleanup (cherry picked from commit 8357dedbbf7213b7d0d18f1026145b9a5b92235a)
1 parent b222b10 commit 48361c4

File tree

1 file changed

+32
-64
lines changed

1 file changed

+32
-64
lines changed

Diff for: winpr/libwinpr/sspi/NTLM/ntlm_message.c

+32-64
Original file line numberDiff line numberDiff line change
@@ -367,12 +367,16 @@ SECURITY_STATUS ntlm_write_NegotiateMessage(NTLM_CONTEXT* context, PSecBuffer bu
367367

368368
SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buffer)
369369
{
370+
SECURITY_STATUS status = SEC_E_INVALID_TOKEN;
370371
wStream* s;
371-
int length;
372-
PBYTE StartOffset;
373-
PBYTE PayloadOffset;
372+
size_t length;
373+
size_t StartOffset;
374+
size_t PayloadOffset;
374375
NTLM_AV_PAIR* AvTimestamp;
375376
NTLM_CHALLENGE_MESSAGE* message;
377+
if (!context || !buffer)
378+
return SEC_E_INTERNAL_ERROR;
379+
376380
ntlm_generate_client_challenge(context);
377381
message = &context->CHALLENGE_MESSAGE;
378382
ZeroMemory(message, sizeof(NTLM_CHALLENGE_MESSAGE));
@@ -381,88 +385,59 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf
381385
if (!s)
382386
return SEC_E_INTERNAL_ERROR;
383387

384-
StartOffset = Stream_Pointer(s);
388+
StartOffset = Stream_GetPosition(s);
385389

386390
if (ntlm_read_message_header(s, (NTLM_MESSAGE_HEADER*)message) < 0)
387-
{
388-
Stream_Free(s, FALSE);
389-
return SEC_E_INVALID_TOKEN;
390-
}
391+
goto fail;
391392

392393
if (message->MessageType != MESSAGE_TYPE_CHALLENGE)
393-
{
394-
Stream_Free(s, FALSE);
395-
return SEC_E_INVALID_TOKEN;
396-
}
394+
goto fail;
397395

398396
if (ntlm_read_message_fields(s, &(message->TargetName)) < 0) /* TargetNameFields (8 bytes) */
399-
{
400-
Stream_Free(s, FALSE);
401-
return SEC_E_INVALID_TOKEN;
402-
}
397+
goto fail;
403398

404399
if (Stream_GetRemainingLength(s) < 4)
405-
{
406-
Stream_Free(s, FALSE);
407-
return SEC_E_INVALID_TOKEN;
408-
}
400+
goto fail;
409401

410402
Stream_Read_UINT32(s, message->NegotiateFlags); /* NegotiateFlags (4 bytes) */
411403
context->NegotiateFlags = message->NegotiateFlags;
412404

413405
if (Stream_GetRemainingLength(s) < 8)
414-
{
415-
Stream_Free(s, FALSE);
416-
return SEC_E_INVALID_TOKEN;
417-
}
406+
goto fail;
418407

419408
Stream_Read(s, message->ServerChallenge, 8); /* ServerChallenge (8 bytes) */
420409
CopyMemory(context->ServerChallenge, message->ServerChallenge, 8);
421410

422411
if (Stream_GetRemainingLength(s) < 8)
423-
{
424-
Stream_Free(s, FALSE);
425-
return SEC_E_INVALID_TOKEN;
426-
}
412+
goto fail;
427413

428414
Stream_Read(s, message->Reserved, 8); /* Reserved (8 bytes), should be ignored */
429415

430416
if (ntlm_read_message_fields(s, &(message->TargetInfo)) < 0) /* TargetInfoFields (8 bytes) */
431-
{
432-
Stream_Free(s, FALSE);
433-
return SEC_E_INVALID_TOKEN;
434-
}
417+
goto fail;
435418

436419
if (context->NegotiateFlags & NTLMSSP_NEGOTIATE_VERSION)
437420
{
438421
if (ntlm_read_version_info(s, &(message->Version)) < 0) /* Version (8 bytes) */
439-
{
440-
Stream_Free(s, FALSE);
441-
return SEC_E_INVALID_TOKEN;
442-
}
422+
goto fail;
443423
}
444424

445425
/* Payload (variable) */
446-
PayloadOffset = Stream_Pointer(s);
426+
PayloadOffset = Stream_GetPosition(s);
447427

428+
status = SEC_E_INTERNAL_ERROR;
448429
if (message->TargetName.Len > 0)
449430
{
450431
if (ntlm_read_message_fields_buffer(s, &(message->TargetName)) < 0)
451-
{
452-
Stream_Free(s, FALSE);
453-
return SEC_E_INTERNAL_ERROR;
454-
}
432+
goto fail;
455433
}
456434

457435
if (message->TargetInfo.Len > 0)
458436
{
459437
size_t cbAvTimestamp;
460438

461439
if (ntlm_read_message_fields_buffer(s, &(message->TargetInfo)) < 0)
462-
{
463-
Stream_Free(s, FALSE);
464-
return SEC_E_INTERNAL_ERROR;
465-
}
440+
goto fail;
466441

467442
context->ChallengeTargetInfo.pvBuffer = message->TargetInfo.Buffer;
468443
context->ChallengeTargetInfo.cbBuffer = message->TargetInfo.Len;
@@ -474,7 +449,7 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf
474449
PBYTE ptr = ntlm_av_pair_get_value_pointer(AvTimestamp);
475450

476451
if (!ptr)
477-
return SEC_E_INTERNAL_ERROR;
452+
goto fail;
478453

479454
if (context->NTLMv2)
480455
context->UseMIC = TRUE;
@@ -484,14 +459,14 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf
484459
}
485460

486461
length = (PayloadOffset - StartOffset) + message->TargetName.Len + message->TargetInfo.Len;
462+
if (length > buffer->cbBuffer)
463+
goto fail;
487464

488465
if (!sspi_SecBufferAlloc(&context->ChallengeMessage, length))
489-
{
490-
Stream_Free(s, FALSE);
491-
return SEC_E_INTERNAL_ERROR;
492-
}
466+
goto fail;
493467

494-
CopyMemory(context->ChallengeMessage.pvBuffer, StartOffset, length);
468+
if (context->ChallengeMessage.pvBuffer)
469+
CopyMemory(context->ChallengeMessage.pvBuffer, Stream_Buffer(s) + StartOffset, length);
495470
#ifdef WITH_DEBUG_NTLM
496471
WLog_DBG(TAG, "CHALLENGE_MESSAGE (length = %d)", length);
497472
winpr_HexDump(TAG, WLOG_DEBUG, context->ChallengeMessage.pvBuffer,
@@ -517,10 +492,7 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf
517492
if (context->NTLMv2)
518493
{
519494
if (ntlm_construct_authenticate_target_info(context) < 0)
520-
{
521-
Stream_Free(s, FALSE);
522-
return SEC_E_INTERNAL_ERROR;
523-
}
495+
goto fail;
524496

525497
sspi_SecBufferFree(&context->ChallengeTargetInfo);
526498
context->ChallengeTargetInfo.pvBuffer = context->AuthenticateTargetInfo.pvBuffer;
@@ -530,16 +502,10 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf
530502
ntlm_generate_timestamp(context); /* Timestamp */
531503

532504
if (ntlm_compute_lm_v2_response(context) < 0) /* LmChallengeResponse */
533-
{
534-
Stream_Free(s, FALSE);
535-
return SEC_E_INTERNAL_ERROR;
536-
}
505+
goto fail;
537506

538507
if (ntlm_compute_ntlm_v2_response(context) < 0) /* NtChallengeResponse */
539-
{
540-
Stream_Free(s, FALSE);
541-
return SEC_E_INTERNAL_ERROR;
542-
}
508+
goto fail;
543509

544510
ntlm_generate_key_exchange_key(context); /* KeyExchangeKey */
545511
ntlm_generate_random_session_key(context); /* RandomSessionKey */
@@ -579,8 +545,10 @@ SECURITY_STATUS ntlm_read_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buf
579545
#endif
580546
context->state = NTLM_STATE_AUTHENTICATE;
581547
ntlm_free_message_fields_buffer(&(message->TargetName));
548+
status = SEC_I_CONTINUE_NEEDED;
549+
fail:
582550
Stream_Free(s, FALSE);
583-
return SEC_I_CONTINUE_NEEDED;
551+
return status;
584552
}
585553

586554
SECURITY_STATUS ntlm_write_ChallengeMessage(NTLM_CONTEXT* context, PSecBuffer buffer)

0 commit comments

Comments
 (0)