Fixed OOB read in ntlm_av_pair_get

CVE-2020-11097 thanks to @antonio-morales for finding this.

(cherry picked from commit 58a3122250d54de3a944c487776bcd4d1da4721e)

diff --git a/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c b/winpr/libwinpr/sspi/NTLM/ntlm_av_pairs.c
index aa873db..47669bb 100644 (file)
--- 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 */