From 7bcfb41489903543546d25ec13f8c58f36a147b3 Mon Sep 17 00:00:00 2001 From: Tomas Mraz Date: Mon, 5 Aug 2024 16:51:56 +0200 Subject: [PATCH] ossl_print_attribute_value(): use a sequence value only if type is a sequence Move the switch to print a distinguished name inside the switch by the printed attribute type, otherwise a malformed attribute will cause a crash. Updated the fuzz corpora with the testcase Reviewed-by: Matt Caswell Reviewed-by: Tom Cosgrove (Merged from https://github.com/openssl/openssl/pull/25087) --- crypto/x509/x_attrib.c | 75 ++++++++++++++++++++---------------------- fuzz/corpora | 2 +- 2 files changed, 36 insertions(+), 41 deletions(-) diff --git a/crypto/x509/x_attrib.c b/crypto/x509/x_attrib.c index bef44dd4ce..b413a27917 100644 --- a/crypto/x509/x_attrib.c +++ b/crypto/x509/x_attrib.c @@ -99,45 +99,6 @@ int ossl_print_attribute_value(BIO *out, X509_NAME *xn = NULL; int64_t int_val; - /* - * This switch-case is only for syntaxes that are not encoded as a single - * primitively-constructed value universal ASN.1 type. - */ - switch (obj_nid) { - case NID_undef: /* Unrecognized OID. */ - break; - /* Attribute types with DN syntax. */ - case NID_member: - case NID_roleOccupant: - case NID_seeAlso: - case NID_manager: - case NID_documentAuthor: - case NID_secretary: - case NID_associatedName: - case NID_dITRedirect: - case NID_owner: - /* - * d2i_ functions increment the ppin pointer. See doc/man3/d2i_X509.pod. - * This preserves the original pointer. We don't want to corrupt this - * value. - */ - value = av->value.sequence->data; - xn = d2i_X509_NAME(NULL, - (const unsigned char**)&value, - av->value.sequence->length); - if (xn == NULL) { - BIO_puts(out, "(COULD NOT DECODE DISTINGUISHED NAME)\n"); - return 0; - } - if (X509_NAME_print_ex(out, xn, indent, XN_FLAG_SEP_CPLUS_SPC) <= 0) - return 0; - X509_NAME_free(xn); - return 1; - - default: - break; - } - switch (av->type) { case V_ASN1_BOOLEAN: if (av->value.boolean) { @@ -176,7 +137,7 @@ int ossl_print_attribute_value(BIO *out, if (BIO_printf(out, "%*s", indent, "") < 0) return 0; return print_oid(out, av->value.object); - + /* * ObjectDescriptor is an IMPLICIT GraphicString, but GeneralString is a * superset supported by OpenSSL, so we will use that anywhere a @@ -204,6 +165,40 @@ int ossl_print_attribute_value(BIO *out, /* TIME would go here. */ case V_ASN1_SEQUENCE: + switch (obj_nid) { + case NID_undef: /* Unrecognized OID. */ + break; + /* Attribute types with DN syntax. */ + case NID_member: + case NID_roleOccupant: + case NID_seeAlso: + case NID_manager: + case NID_documentAuthor: + case NID_secretary: + case NID_associatedName: + case NID_dITRedirect: + case NID_owner: + /* + * d2i_ functions increment the ppin pointer. See doc/man3/d2i_X509.pod. + * This preserves the original pointer. We don't want to corrupt this + * value. + */ + value = av->value.sequence->data; + xn = d2i_X509_NAME(NULL, + (const unsigned char **)&value, + av->value.sequence->length); + if (xn == NULL) { + BIO_puts(out, "(COULD NOT DECODE DISTINGUISHED NAME)\n"); + return 0; + } + if (X509_NAME_print_ex(out, xn, indent, XN_FLAG_SEP_CPLUS_SPC) <= 0) + return 0; + X509_NAME_free(xn); + return 1; + + default: + break; + } return ASN1_parse_dump(out, av->value.sequence->data, av->value.sequence->length, indent, 1) > 0; diff --git a/fuzz/corpora b/fuzz/corpora index 9f76670613..e77927ec1b 160000 --- a/fuzz/corpora +++ b/fuzz/corpora @@ -1 +1 @@ -Subproject commit 9f7667061314ecf9a287ce1c9702073ca1e345e3 +Subproject commit e77927ec1b3b3633d6fd2d905df6fdc59810c9e0