| From f960d81215ebf3f65e03d4d5d857fb9b666d6920 Mon Sep 17 00:00:00 2001 |
| From: Matt Caswell <matt@openssl.org> |
| Date: Wed, 11 Nov 2020 16:12:58 +0000 |
| Subject: [PATCH] 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> |
| --- |
| crypto/x509v3/v3_genn.c | 45 ++++++++++++++++++++++++++++++++++++++--- |
| 1 file changed, 42 insertions(+), 3 deletions(-) |
| |
| diff --git a/crypto/x509v3/v3_genn.c b/crypto/x509v3/v3_genn.c |
| index b483f35a9e..6f0a347cce 100644 |
| --- a/crypto/x509v3/v3_genn.c |
| +++ b/crypto/x509v3/v3_genn.c |
| @@ -58,6 +58,37 @@ GENERAL_NAME *GENERAL_NAME_dup(GENERAL_NAME *a) |
| (char *)a); |
| } |
| |
| +static int edipartyname_cmp(const EDIPARTYNAME *a, const EDIPARTYNAME *b) |
| +{ |
| + int res; |
| + |
| + if (a == NULL || b == NULL) { |
| + /* |
| + * Shouldn't be possible in a valid GENERAL_NAME, but we handle it |
| + * anyway. OTHERNAME_cmp treats NULL != NULL so we do the same here |
| + */ |
| + return -1; |
| + } |
| + if (a->nameAssigner == NULL && b->nameAssigner != NULL) |
| + return -1; |
| + if (a->nameAssigner != NULL && b->nameAssigner == NULL) |
| + return 1; |
| + /* If we get here then both have nameAssigner set, or both unset */ |
| + if (a->nameAssigner != NULL) { |
| + res = ASN1_STRING_cmp(a->nameAssigner, b->nameAssigner); |
| + if (res != 0) |
| + return res; |
| + } |
| + /* |
| + * partyName is required, so these should never be NULL. We treat it in |
| + * the same way as the a == NULL || b == NULL case above |
| + */ |
| + if (a->partyName == NULL || b->partyName == NULL) |
| + return -1; |
| + |
| + return ASN1_STRING_cmp(a->partyName, b->partyName); |
| +} |
| + |
| /* Returns 0 if they are equal, != 0 otherwise. */ |
| int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) |
| { |
| @@ -67,8 +98,11 @@ int GENERAL_NAME_cmp(GENERAL_NAME *a, GENERAL_NAME *b) |
| return -1; |
| switch (a->type) { |
| case GEN_X400: |
| + result = ASN1_TYPE_cmp(a->d.x400Address, b->d.x400Address); |
| + break; |
| + |
| case GEN_EDIPARTY: |
| - result = ASN1_TYPE_cmp(a->d.other, b->d.other); |
| + result = edipartyname_cmp(a->d.ediPartyName, b->d.ediPartyName); |
| break; |
| |
| case GEN_OTHERNAME: |
| @@ -115,8 +149,11 @@ void GENERAL_NAME_set0_value(GENERAL_NAME *a, int type, void *value) |
| { |
| switch (type) { |
| case GEN_X400: |
| + a->d.x400Address = value; |
| + break; |
| + |
| case GEN_EDIPARTY: |
| - a->d.other = value; |
| + a->d.ediPartyName = value; |
| break; |
| |
| case GEN_OTHERNAME: |
| @@ -150,8 +187,10 @@ void *GENERAL_NAME_get0_value(const GENERAL_NAME *a, int *ptype) |
| *ptype = a->type; |
| switch (a->type) { |
| case GEN_X400: |
| + return a->d.x400Address; |
| + |
| case GEN_EDIPARTY: |
| - return a->d.other; |
| + return a->d.ediPartyName; |
| |
| case GEN_OTHERNAME: |
| return a->d.otherName; |
| -- |
| 2.17.1 |