From 58a3122250d54de3a944c487776bcd4d1da4721e Mon Sep 17 00:00:00 2001 From: akallabeth Date: Tue, 2 Jun 2020 12:26:40 +0200 Subject: [PATCH] Fixed OOB read in ntlm_av_pair_get CVE-2020-11097 thanks to @antonio-morales for finding this. --- winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c | 146 ++++++++++++++++++----- 1 file changed, 113 insertions(+), 33 deletions(-) diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c index aa873db5e..47669bb2e 100644 --- a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c +++ b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c @@ -39,13 +39,50 @@ #include "../../log.h" #define TAG WINPR_TAG("sspi.NTLM") -static const char* const AV_PAIR_STRINGS[] = { - "MsvAvEOL", "MsvAvNbComputerName", "MsvAvNbDomainName", "MsvAvDnsComputerName", - "MsvAvDnsDomainName", "MsvAvDnsTreeName", "MsvAvFlags", "MsvAvTimestamp", - "MsvAvRestrictions", "MsvAvTargetName", "MsvChannelBindings" -}; +static BOOL ntlm_av_pair_get_next_offset(const NTLM_AV_PAIR* pAvPair, size_t size, size_t* pOffset); -static BOOL ntlm_av_pair_check(NTLM_AV_PAIR* pAvPair, size_t cbAvPair); +static BOOL ntlm_av_pair_check_data(const NTLM_AV_PAIR* pAvPair, size_t cbAvPair, size_t size) +{ + size_t offset; + if (!pAvPair || cbAvPair < sizeof(NTLM_AV_PAIR) + size) + return FALSE; + if (!ntlm_av_pair_get_next_offset(pAvPair, cbAvPair, &offset)) + return FALSE; + return cbAvPair >= offset; +} + +static const char* get_av_pair_string(UINT16 pair) +{ + switch (pair) + { + case MsvAvEOL: + return "MsvAvEOL"; + case MsvAvNbComputerName: + return "MsvAvNbComputerName"; + case MsvAvNbDomainName: + return "MsvAvNbDomainName"; + case MsvAvDnsComputerName: + return "MsvAvDnsComputerName"; + case MsvAvDnsDomainName: + return "MsvAvDnsDomainName"; + case MsvAvDnsTreeName: + return "MsvAvDnsTreeName"; + case MsvAvFlags: + return "MsvAvFlags"; + case MsvAvTimestamp: + return "MsvAvTimestamp"; + case MsvAvSingleHost: + return "MsvAvSingleHost"; + case MsvAvTargetName: + return "MsvAvTargetName"; + case MsvChannelBindings: + return "MsvChannelBindings"; + default: + return "UNKNOWN"; + } +} + +static BOOL ntlm_av_pair_check(const NTLM_AV_PAIR* pAvPair, size_t cbAvPair); static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPairList, size_t* pcbAvPairList); static INLINE void ntlm_av_pair_set_id(NTLM_AV_PAIR* pAvPair, UINT16 id) @@ -70,13 +107,19 @@ static BOOL ntlm_av_pair_list_init(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairLis return TRUE; } -static INLINE UINT16 ntlm_av_pair_get_id(const NTLM_AV_PAIR* pAvPair) +static INLINE BOOL ntlm_av_pair_get_id(const NTLM_AV_PAIR* pAvPair, size_t size, UINT16* pair) { UINT16 AvId; + if (!pAvPair || !pair) + return FALSE; + + if (size < sizeof(NTLM_AV_PAIR)) + return FALSE; Data_Read_UINT16(&pAvPair->AvId, AvId); - return AvId; + *pair = AvId; + return TRUE; } ULONG ntlm_av_pair_list_length(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList) @@ -91,17 +134,24 @@ ULONG ntlm_av_pair_list_length(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList) return ((PBYTE)pAvPair - (PBYTE)pAvPairList) + sizeof(NTLM_AV_PAIR); } -static INLINE SIZE_T ntlm_av_pair_get_len(const NTLM_AV_PAIR* pAvPair) +static INLINE BOOL ntlm_av_pair_get_len(const NTLM_AV_PAIR* pAvPair, size_t size, size_t* pAvLen) { UINT16 AvLen; + if (!pAvPair) + return FALSE; + + if (size < sizeof(NTLM_AV_PAIR)) + return FALSE; Data_Read_UINT16(&pAvPair->AvLen, AvLen); - return AvLen; + *pAvLen = AvLen; + return TRUE; } void ntlm_print_av_pair_list(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList) { + UINT16 pair; size_t cbAvPair = cbAvPairList; NTLM_AV_PAIR* pAvPair = pAvPairList; @@ -110,13 +160,13 @@ void ntlm_print_av_pair_list(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList) WLog_INFO(TAG, "AV_PAIRs ="); - while (pAvPair && ntlm_av_pair_get_id(pAvPair) != MsvAvEOL) + while (pAvPair && ntlm_av_pair_get_id(pAvPair, cbAvPair, &pair) && (pair != MsvAvEOL)) { - WLog_INFO(TAG, "\t%s AvId: %" PRIu16 " AvLen: %" PRIu16 "", - AV_PAIR_STRINGS[ntlm_av_pair_get_id(pAvPair)], ntlm_av_pair_get_id(pAvPair), - ntlm_av_pair_get_len(pAvPair)); - winpr_HexDump(TAG, WLOG_INFO, ntlm_av_pair_get_value_pointer(pAvPair), - ntlm_av_pair_get_len(pAvPair)); + size_t cbLen = 0; + ntlm_av_pair_get_len(pAvPair, cbAvPair, &cbLen); + + WLog_INFO(TAG, "\t%s AvId: %" PRIu16 " AvLen: %" PRIu16 "", get_av_pair_string(pair), pair); + winpr_HexDump(TAG, WLOG_INFO, ntlm_av_pair_get_value_pointer(pAvPair), cbLen); pAvPair = ntlm_av_pair_next(pAvPair, &cbAvPair); } @@ -133,16 +183,21 @@ PBYTE ntlm_av_pair_get_value_pointer(NTLM_AV_PAIR* pAvPair) return (PBYTE)pAvPair + sizeof(NTLM_AV_PAIR); } -static size_t ntlm_av_pair_get_next_offset(NTLM_AV_PAIR* pAvPair) +static BOOL ntlm_av_pair_get_next_offset(const NTLM_AV_PAIR* pAvPair, size_t size, size_t* pOffset) { - return ntlm_av_pair_get_len(pAvPair) + sizeof(NTLM_AV_PAIR); + size_t avLen; + if (!pOffset) + return FALSE; + + if (!ntlm_av_pair_get_len(pAvPair, size, &avLen)) + return FALSE; + *pOffset = avLen + sizeof(NTLM_AV_PAIR); + return TRUE; } -static BOOL ntlm_av_pair_check(NTLM_AV_PAIR* pAvPair, size_t cbAvPair) +static BOOL ntlm_av_pair_check(const NTLM_AV_PAIR* pAvPair, size_t cbAvPair) { - if (!pAvPair || cbAvPair < sizeof(NTLM_AV_PAIR)) - return FALSE; - return cbAvPair >= ntlm_av_pair_get_next_offset(pAvPair); + return ntlm_av_pair_check_data(pAvPair, cbAvPair, 0); } static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPair, size_t* pcbAvPair) @@ -154,7 +209,9 @@ static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPair, size_t* pcbAvPair) if (!ntlm_av_pair_check(pAvPair, *pcbAvPair)) return NULL; - offset = ntlm_av_pair_get_next_offset(pAvPair); + if (!ntlm_av_pair_get_next_offset(pAvPair, *pcbAvPair, &offset)) + return NULL; + *pcbAvPair -= offset; return (NTLM_AV_PAIR*)((PBYTE)pAvPair + offset); } @@ -162,16 +219,15 @@ static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPair, size_t* pcbAvPair) NTLM_AV_PAIR* ntlm_av_pair_get(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList, NTLM_AV_ID AvId, size_t* pcbAvPairListRemaining) { + UINT16 id; size_t cbAvPair = cbAvPairList; NTLM_AV_PAIR* pAvPair = pAvPairList; if (!ntlm_av_pair_check(pAvPair, cbAvPair)) pAvPair = NULL; - while (pAvPair) + while (pAvPair && ntlm_av_pair_get_id(pAvPair, cbAvPair, &id)) { - UINT16 id = ntlm_av_pair_get_id(pAvPair); - if (id == AvId) break; if (id == MsvAvEOL) @@ -218,11 +274,20 @@ static BOOL ntlm_av_pair_add(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList, NTL static BOOL ntlm_av_pair_add_copy(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList, NTLM_AV_PAIR* pAvPair, size_t cbAvPair) { + UINT16 pair; + size_t avLen; + if (!ntlm_av_pair_check(pAvPair, cbAvPair)) return FALSE; - return ntlm_av_pair_add(pAvPairList, cbAvPairList, ntlm_av_pair_get_id(pAvPair), - ntlm_av_pair_get_value_pointer(pAvPair), ntlm_av_pair_get_len(pAvPair)); + if (!ntlm_av_pair_get_id(pAvPair, cbAvPair, &pair)) + return FALSE; + + if (!ntlm_av_pair_get_len(pAvPair, cbAvPair, &avLen)) + return FALSE; + + return ntlm_av_pair_add(pAvPairList, cbAvPairList, pair, + ntlm_av_pair_get_value_pointer(pAvPair), avLen); } static int ntlm_get_target_computer_name(PUNICODE_STRING pName, COMPUTER_NAME_FORMAT type) @@ -500,32 +565,47 @@ int ntlm_construct_authenticate_target_info(NTLM_CONTEXT* context) if (AvNbDomainName) { + size_t avLen; + if (!ntlm_av_pair_get_len(AvNbDomainName, cbAvNbDomainName, &avLen)) + goto fail; AvPairsCount++; /* MsvAvNbDomainName */ - AvPairsValueLength += ntlm_av_pair_get_len(AvNbDomainName); + AvPairsValueLength += avLen; } if (AvNbComputerName) { + size_t avLen; + if (!ntlm_av_pair_get_len(AvNbComputerName, cbAvNbComputerName, &avLen)) + goto fail; AvPairsCount++; /* MsvAvNbComputerName */ - AvPairsValueLength += ntlm_av_pair_get_len(AvNbComputerName); + AvPairsValueLength += avLen; } if (AvDnsDomainName) { + size_t avLen; + if (!ntlm_av_pair_get_len(AvDnsDomainName, cbAvDnsDomainName, &avLen)) + goto fail; AvPairsCount++; /* MsvAvDnsDomainName */ - AvPairsValueLength += ntlm_av_pair_get_len(AvDnsDomainName); + AvPairsValueLength += avLen; } if (AvDnsComputerName) { + size_t avLen; + if (!ntlm_av_pair_get_len(AvDnsComputerName, cbAvDnsComputerName, &avLen)) + goto fail; AvPairsCount++; /* MsvAvDnsComputerName */ - AvPairsValueLength += ntlm_av_pair_get_len(AvDnsComputerName); + AvPairsValueLength += avLen; } if (AvDnsTreeName) { + size_t avLen; + if (!ntlm_av_pair_get_len(AvDnsTreeName, cbAvDnsTreeName, &avLen)) + goto fail; AvPairsCount++; /* MsvAvDnsTreeName */ - AvPairsValueLength += ntlm_av_pair_get_len(AvDnsTreeName); + AvPairsValueLength += avLen; } AvPairsCount++; /* MsvAvTimestamp */