Skip to content

Commit f960d81

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 aa0ad20 commit f960d81

File tree

1 file changed

+42
-3
lines changed

1 file changed

+42
-3
lines changed

crypto/x509v3/v3_genn.c

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,37 @@ GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a)
5858
(char *)a);
5959
}
6060

61+
static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b)
62+
{
63+
int res;
64+
65+
if (a == NULL || b == NULL) {
66+
/*
67+
* Shouldn't be possible in a valid GENERAL_NAME, but we handle it
68+
* anyway. OTHERNAME_cmp treats NULL != NULL so we do the same here
69+
*/
70+
return -1;
71+
}
72+
if (a->nameAssigner == NULL && b->nameAssigner != NULL)
73+
return -1;
74+
if (a->nameAssigner != NULL && b->nameAssigner == NULL)
75+
return 1;
76+
/* If we get here then both have nameAssigner set, or both unset */
77+
if (a->nameAssigner != NULL) {
78+
res = ASN1_STRING_cmp(a->nameAssigner, b->nameAssigner);
79+
if (res != 0)
80+
return res;
81+
}
82+
/*
83+
* partyName is required, so these should never be NULL. We treat it in
84+
* the same way as the a == NULL || b == NULL case above
85+
*/
86+
if (a->partyName == NULL || b->partyName == NULL)
87+
return -1;
88+
89+
return ASN1_STRING_cmp(a->partyName, b->partyName);
90+
}
91+
6192
/* Returns 0 if they are equal, != 0 otherwise. */
6293
int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b)
6394
{
@@ -67,8 +98,11 @@ int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b)
6798
return -1;
6899
switch (a->type) {
69100
case GEN_X400:
101+
result = ASN1_TYPE_cmp(a->d.x400Address, b->d.x400Address);
102+
break;
103+
70104
case GEN_EDIPARTY:
71-
result = ASN1_TYPE_cmp(a->d.other, b->d.other);
105+
result = edipartyname_cmp(a->d.ediPartyName, b->d.ediPartyName);
72106
break;
73107

74108
case GEN_OTHERNAME:
@@ -115,8 +149,11 @@ void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value)
115149
{
116150
switch (type) {
117151
case GEN_X400:
152+
a->d.x400Address = value;
153+
break;
154+
118155
case GEN_EDIPARTY:
119-
a->d.other = value;
156+
a->d.ediPartyName = value;
120157
break;
121158

122159
case GEN_OTHERNAME:
@@ -150,8 +187,10 @@ void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *ptype)
150187
*ptype = a->type;
151188
switch (a->type) {
152189
case GEN_X400:
190+
return a->d.x400Address;
191+
153192
case GEN_EDIPARTY:
154-
return a->d.other;
193+
return a->d.ediPartyName;
155194

156195
case GEN_OTHERNAME:
157196
return a->d.otherName;

0 commit comments

Comments
 (0)