Skip to content

Commit 58a3122

Browse files
committed
Fixed OOB read in ntlm_av_pair_get
CVE-2020-11097 thanks to @antonio-morales for finding this.
1 parent 1c6a692 commit 58a3122

File tree

1 file changed

+113
-33
lines changed

1 file changed

+113
-33
lines changed

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

+113-33
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,50 @@
3939
#include "../../log.h"
4040
#define TAG WINPR_TAG("sspi.NTLM")
4141

42-
static const char* const AV_PAIR_STRINGS[] = {
43-
"MsvAvEOL", "MsvAvNbComputerName", "MsvAvNbDomainName", "MsvAvDnsComputerName",
44-
"MsvAvDnsDomainName", "MsvAvDnsTreeName", "MsvAvFlags", "MsvAvTimestamp",
45-
"MsvAvRestrictions", "MsvAvTargetName", "MsvChannelBindings"
46-
};
42+
static BOOL ntlm_av_pair_get_next_offset(const NTLM_AV_PAIR* pAvPair, size_t size, size_t* pOffset);
4743

48-
static BOOL ntlm_av_pair_check(NTLM_AV_PAIR* pAvPair, size_t cbAvPair);
44+
static BOOL ntlm_av_pair_check_data(const NTLM_AV_PAIR* pAvPair, size_t cbAvPair, size_t size)
45+
{
46+
size_t offset;
47+
if (!pAvPair || cbAvPair < sizeof(NTLM_AV_PAIR) + size)
48+
return FALSE;
49+
if (!ntlm_av_pair_get_next_offset(pAvPair, cbAvPair, &offset))
50+
return FALSE;
51+
return cbAvPair >= offset;
52+
}
53+
54+
static const char* get_av_pair_string(UINT16 pair)
55+
{
56+
switch (pair)
57+
{
58+
case MsvAvEOL:
59+
return "MsvAvEOL";
60+
case MsvAvNbComputerName:
61+
return "MsvAvNbComputerName";
62+
case MsvAvNbDomainName:
63+
return "MsvAvNbDomainName";
64+
case MsvAvDnsComputerName:
65+
return "MsvAvDnsComputerName";
66+
case MsvAvDnsDomainName:
67+
return "MsvAvDnsDomainName";
68+
case MsvAvDnsTreeName:
69+
return "MsvAvDnsTreeName";
70+
case MsvAvFlags:
71+
return "MsvAvFlags";
72+
case MsvAvTimestamp:
73+
return "MsvAvTimestamp";
74+
case MsvAvSingleHost:
75+
return "MsvAvSingleHost";
76+
case MsvAvTargetName:
77+
return "MsvAvTargetName";
78+
case MsvChannelBindings:
79+
return "MsvChannelBindings";
80+
default:
81+
return "UNKNOWN";
82+
}
83+
}
84+
85+
static BOOL ntlm_av_pair_check(const NTLM_AV_PAIR* pAvPair, size_t cbAvPair);
4986
static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPairList, size_t* pcbAvPairList);
5087

5188
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
70107
return TRUE;
71108
}
72109

73-
static INLINE UINT16 ntlm_av_pair_get_id(const NTLM_AV_PAIR* pAvPair)
110+
static INLINE BOOL ntlm_av_pair_get_id(const NTLM_AV_PAIR* pAvPair, size_t size, UINT16* pair)
74111
{
75112
UINT16 AvId;
113+
if (!pAvPair || !pair)
114+
return FALSE;
115+
116+
if (size < sizeof(NTLM_AV_PAIR))
117+
return FALSE;
76118

77119
Data_Read_UINT16(&pAvPair->AvId, AvId);
78120

79-
return AvId;
121+
*pair = AvId;
122+
return TRUE;
80123
}
81124

82125
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)
91134
return ((PBYTE)pAvPair - (PBYTE)pAvPairList) + sizeof(NTLM_AV_PAIR);
92135
}
93136

94-
static INLINE SIZE_T ntlm_av_pair_get_len(const NTLM_AV_PAIR* pAvPair)
137+
static INLINE BOOL ntlm_av_pair_get_len(const NTLM_AV_PAIR* pAvPair, size_t size, size_t* pAvLen)
95138
{
96139
UINT16 AvLen;
140+
if (!pAvPair)
141+
return FALSE;
142+
143+
if (size < sizeof(NTLM_AV_PAIR))
144+
return FALSE;
97145

98146
Data_Read_UINT16(&pAvPair->AvLen, AvLen);
99147

100-
return AvLen;
148+
*pAvLen = AvLen;
149+
return TRUE;
101150
}
102151

103152
void ntlm_print_av_pair_list(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList)
104153
{
154+
UINT16 pair;
105155
size_t cbAvPair = cbAvPairList;
106156
NTLM_AV_PAIR* pAvPair = pAvPairList;
107157

@@ -110,13 +160,13 @@ void ntlm_print_av_pair_list(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList)
110160

111161
WLog_INFO(TAG, "AV_PAIRs =");
112162

113-
while (pAvPair && ntlm_av_pair_get_id(pAvPair) != MsvAvEOL)
163+
while (pAvPair && ntlm_av_pair_get_id(pAvPair, cbAvPair, &pair) && (pair != MsvAvEOL))
114164
{
115-
WLog_INFO(TAG, "\t%s AvId: %" PRIu16 " AvLen: %" PRIu16 "",
116-
AV_PAIR_STRINGS[ntlm_av_pair_get_id(pAvPair)], ntlm_av_pair_get_id(pAvPair),
117-
ntlm_av_pair_get_len(pAvPair));
118-
winpr_HexDump(TAG, WLOG_INFO, ntlm_av_pair_get_value_pointer(pAvPair),
119-
ntlm_av_pair_get_len(pAvPair));
165+
size_t cbLen = 0;
166+
ntlm_av_pair_get_len(pAvPair, cbAvPair, &cbLen);
167+
168+
WLog_INFO(TAG, "\t%s AvId: %" PRIu16 " AvLen: %" PRIu16 "", get_av_pair_string(pair), pair);
169+
winpr_HexDump(TAG, WLOG_INFO, ntlm_av_pair_get_value_pointer(pAvPair), cbLen);
120170

121171
pAvPair = ntlm_av_pair_next(pAvPair, &cbAvPair);
122172
}
@@ -133,16 +183,21 @@ PBYTE ntlm_av_pair_get_value_pointer(NTLM_AV_PAIR* pAvPair)
133183
return (PBYTE)pAvPair + sizeof(NTLM_AV_PAIR);
134184
}
135185

136-
static size_t ntlm_av_pair_get_next_offset(NTLM_AV_PAIR* pAvPair)
186+
static BOOL ntlm_av_pair_get_next_offset(const NTLM_AV_PAIR* pAvPair, size_t size, size_t* pOffset)
137187
{
138-
return ntlm_av_pair_get_len(pAvPair) + sizeof(NTLM_AV_PAIR);
188+
size_t avLen;
189+
if (!pOffset)
190+
return FALSE;
191+
192+
if (!ntlm_av_pair_get_len(pAvPair, size, &avLen))
193+
return FALSE;
194+
*pOffset = avLen + sizeof(NTLM_AV_PAIR);
195+
return TRUE;
139196
}
140197

141-
static BOOL ntlm_av_pair_check(NTLM_AV_PAIR* pAvPair, size_t cbAvPair)
198+
static BOOL ntlm_av_pair_check(const NTLM_AV_PAIR* pAvPair, size_t cbAvPair)
142199
{
143-
if (!pAvPair || cbAvPair < sizeof(NTLM_AV_PAIR))
144-
return FALSE;
145-
return cbAvPair >= ntlm_av_pair_get_next_offset(pAvPair);
200+
return ntlm_av_pair_check_data(pAvPair, cbAvPair, 0);
146201
}
147202

148203
static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPair, size_t* pcbAvPair)
@@ -154,24 +209,25 @@ static NTLM_AV_PAIR* ntlm_av_pair_next(NTLM_AV_PAIR* pAvPair, size_t* pcbAvPair)
154209
if (!ntlm_av_pair_check(pAvPair, *pcbAvPair))
155210
return NULL;
156211

157-
offset = ntlm_av_pair_get_next_offset(pAvPair);
212+
if (!ntlm_av_pair_get_next_offset(pAvPair, *pcbAvPair, &offset))
213+
return NULL;
214+
158215
*pcbAvPair -= offset;
159216
return (NTLM_AV_PAIR*)((PBYTE)pAvPair + offset);
160217
}
161218

162219
NTLM_AV_PAIR* ntlm_av_pair_get(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList, NTLM_AV_ID AvId,
163220
size_t* pcbAvPairListRemaining)
164221
{
222+
UINT16 id;
165223
size_t cbAvPair = cbAvPairList;
166224
NTLM_AV_PAIR* pAvPair = pAvPairList;
167225

168226
if (!ntlm_av_pair_check(pAvPair, cbAvPair))
169227
pAvPair = NULL;
170228

171-
while (pAvPair)
229+
while (pAvPair && ntlm_av_pair_get_id(pAvPair, cbAvPair, &id))
172230
{
173-
UINT16 id = ntlm_av_pair_get_id(pAvPair);
174-
175231
if (id == AvId)
176232
break;
177233
if (id == MsvAvEOL)
@@ -218,11 +274,20 @@ static BOOL ntlm_av_pair_add(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList, NTL
218274
static BOOL ntlm_av_pair_add_copy(NTLM_AV_PAIR* pAvPairList, size_t cbAvPairList,
219275
NTLM_AV_PAIR* pAvPair, size_t cbAvPair)
220276
{
277+
UINT16 pair;
278+
size_t avLen;
279+
221280
if (!ntlm_av_pair_check(pAvPair, cbAvPair))
222281
return FALSE;
223282

224-
return ntlm_av_pair_add(pAvPairList, cbAvPairList, ntlm_av_pair_get_id(pAvPair),
225-
ntlm_av_pair_get_value_pointer(pAvPair), ntlm_av_pair_get_len(pAvPair));
283+
if (!ntlm_av_pair_get_id(pAvPair, cbAvPair, &pair))
284+
return FALSE;
285+
286+
if (!ntlm_av_pair_get_len(pAvPair, cbAvPair, &avLen))
287+
return FALSE;
288+
289+
return ntlm_av_pair_add(pAvPairList, cbAvPairList, pair,
290+
ntlm_av_pair_get_value_pointer(pAvPair), avLen);
226291
}
227292

228293
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)
500565

501566
if (AvNbDomainName)
502567
{
568+
size_t avLen;
569+
if (!ntlm_av_pair_get_len(AvNbDomainName, cbAvNbDomainName, &avLen))
570+
goto fail;
503571
AvPairsCount++; /* MsvAvNbDomainName */
504-
AvPairsValueLength += ntlm_av_pair_get_len(AvNbDomainName);
572+
AvPairsValueLength += avLen;
505573
}
506574

507575
if (AvNbComputerName)
508576
{
577+
size_t avLen;
578+
if (!ntlm_av_pair_get_len(AvNbComputerName, cbAvNbComputerName, &avLen))
579+
goto fail;
509580
AvPairsCount++; /* MsvAvNbComputerName */
510-
AvPairsValueLength += ntlm_av_pair_get_len(AvNbComputerName);
581+
AvPairsValueLength += avLen;
511582
}
512583

513584
if (AvDnsDomainName)
514585
{
586+
size_t avLen;
587+
if (!ntlm_av_pair_get_len(AvDnsDomainName, cbAvDnsDomainName, &avLen))
588+
goto fail;
515589
AvPairsCount++; /* MsvAvDnsDomainName */
516-
AvPairsValueLength += ntlm_av_pair_get_len(AvDnsDomainName);
590+
AvPairsValueLength += avLen;
517591
}
518592

519593
if (AvDnsComputerName)
520594
{
595+
size_t avLen;
596+
if (!ntlm_av_pair_get_len(AvDnsComputerName, cbAvDnsComputerName, &avLen))
597+
goto fail;
521598
AvPairsCount++; /* MsvAvDnsComputerName */
522-
AvPairsValueLength += ntlm_av_pair_get_len(AvDnsComputerName);
599+
AvPairsValueLength += avLen;
523600
}
524601

525602
if (AvDnsTreeName)
526603
{
604+
size_t avLen;
605+
if (!ntlm_av_pair_get_len(AvDnsTreeName, cbAvDnsTreeName, &avLen))
606+
goto fail;
527607
AvPairsCount++; /* MsvAvDnsTreeName */
528-
AvPairsValueLength += ntlm_av_pair_get_len(AvDnsTreeName);
608+
AvPairsValueLength += avLen;
529609
}
530610

531611
AvPairsCount++; /* MsvAvTimestamp */

0 commit comments

Comments
 (0)