Skip to content

Commit 2154ab8

Browse files
committed
Correctly compare EdiPartyName in GENERAL_NAME_cmp()
If a GENERAL_NAME field contained EdiPartyName data then it was incorrectly being handled as type "other". This could lead to a segmentation fault. Many thanks to David Benjamin from Google for reporting this issue. CVE-2020-1971 Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
1 parent 33282fd commit 2154ab8

File tree

1 file changed

+42
-3
lines changed

1 file changed

+42
-3
lines changed

Diff for: crypto/x509v3/v3_genn.c

+42-3
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,37 @@ GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a)
108108
(char *)a);
109109
}
110110

111+
static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b)
112+
{
113+
int res;
114+
115+
if (a == NULL || b == NULL) {
116+
/*
117+
* Shouldn't be possible in a valid GENERAL_NAME, but we handle it
118+
* anyway. OTHERNAME_cmp treats NULL != NULL so we do the same here
119+
*/
120+
return -1;
121+
}
122+
if (a->nameAssigner == NULL && b->nameAssigner != NULL)
123+
return -1;
124+
if (a->nameAssigner != NULL && b->nameAssigner == NULL)
125+
return 1;
126+
/* If we get here then both have nameAssigner set, or both unset */
127+
if (a->nameAssigner != NULL) {
128+
res = ASN1_STRING_cmp(a->nameAssigner, b->nameAssigner);
129+
if (res != 0)
130+
return res;
131+
}
132+
/*
133+
* partyName is required, so these should never be NULL. We treat it in
134+
* the same way as the a == NULL || b == NULL case above
135+
*/
136+
if (a->partyName == NULL || b->partyName == NULL)
137+
return -1;
138+
139+
return ASN1_STRING_cmp(a->partyName, b->partyName);
140+
}
141+
111142
/* Returns 0 if they are equal, != 0 otherwise. */
112143
int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b)
113144
{
@@ -117,8 +148,11 @@ int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b)
117148
return -1;
118149
switch (a->type) {
119150
case GEN_X400:
151+
result = ASN1_TYPE_cmp(a->d.x400Address, b->d.x400Address);
152+
break;
153+
120154
case GEN_EDIPARTY:
121-
result = ASN1_TYPE_cmp(a->d.other, b->d.other);
155+
result = edipartyname_cmp(a->d.ediPartyName, b->d.ediPartyName);
122156
break;
123157

124158
case GEN_OTHERNAME:
@@ -165,8 +199,11 @@ void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value)
165199
{
166200
switch (type) {
167201
case GEN_X400:
202+
a->d.x400Address = value;
203+
break;
204+
168205
case GEN_EDIPARTY:
169-
a->d.other = value;
206+
a->d.ediPartyName = value;
170207
break;
171208

172209
case GEN_OTHERNAME:
@@ -200,8 +237,10 @@ void *GENERAL_NAME_get0_value(GENERAL_NAME *a, int *ptype)
200237
*ptype = a->type;
201238
switch (a->type) {
202239
case GEN_X400:
240+
return a->d.x400Address;
241+
203242
case GEN_EDIPARTY:
204-
return a->d.other;
243+
return a->d.ediPartyName;
205244

206245
case GEN_OTHERNAME:
207246
return a->d.otherName;

0 commit comments

Comments
 (0)