From 5990caf7938d08319a5970cb0c1ced3c8c73b972 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 14 Dec 2022 16:18:14 +0000 Subject: [PATCH 1/2] Fix a UAF resulting from a bug in BIO_new_NDEF If the aux->asn1_cb() call fails in BIO_new_NDEF then the "out" BIO will be part of an invalid BIO chain. This causes a "use after free" when the BIO is eventually freed. Based on an original patch by Viktor Dukhovni and an idea from Theo Buehler. Thanks to Octavio Galland for reporting this issue. Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz Signed-off-by: code4lala --- crypto/asn1/bio_ndef.c | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/crypto/asn1/bio_ndef.c b/crypto/asn1/bio_ndef.c index 6222c99074..cf52468fed 100755 --- a/crypto/asn1/bio_ndef.c +++ b/crypto/asn1/bio_ndef.c @@ -49,12 +49,19 @@ static int ndef_suffix(BIO *b, unsigned char **pbuf, int *plen, void *parg); static int ndef_suffix_free(BIO *b, unsigned char **pbuf, int *plen, void *parg); +/* + * On success, the returned BIO owns the input BIO as part of its BIO chain. + * On failure, NULL is returned and the input BIO is owned by the caller. + * + * Unfortunately cannot constify this due to CMS_stream() and PKCS7_stream() + */ BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it) { NDEF_SUPPORT *ndef_aux = NULL; BIO *asn_bio = NULL; const ASN1_AUX *aux = it->funcs; ASN1_STREAM_ARG sarg; + BIO *pop_bio = NULL; if (!aux || !aux->asn1_cb) { ASN1err(ASN1_F_BIO_NEW_NDEF, ASN1_R_STREAMING_NOT_SUPPORTED); @@ -69,21 +76,39 @@ BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it) out = BIO_push(asn_bio, out); if (out == NULL) goto err; + pop_bio = asn_bio; - BIO_asn1_set_prefix(asn_bio, ndef_prefix, ndef_prefix_free); - BIO_asn1_set_suffix(asn_bio, ndef_suffix, ndef_suffix_free); + if (BIO_asn1_set_prefix(asn_bio, ndef_prefix, ndef_prefix_free) <= 0 + || BIO_asn1_set_suffix(asn_bio, ndef_suffix, ndef_suffix_free) <= 0 + || BIO_ctrl(asn_bio, BIO_C_SET_EX_ARG, 0, ndef_aux) <= 0) + goto err; /* - * Now let callback prepends any digest, cipher etc BIOs ASN1 structure - * needs. + * Now let the callback prepend any digest, cipher, etc., that the BIO's + * ASN1 structure needs. */ sarg.out = out; sarg.ndef_bio = NULL; sarg.boundary = NULL; - if (aux->asn1_cb(ASN1_OP_STREAM_PRE, &val, it, &sarg) <= 0) + /* + * The asn1_cb(), must not have mutated asn_bio on error, leaving it in the + * middle of some partially built, but not returned BIO chain. + */ + if (aux->asn1_cb(ASN1_OP_STREAM_PRE, &val, it, &sarg) <= 0) { + /* + * ndef_aux is now owned by asn_bio so we must not free it in the err + * clean up block + */ + ndef_aux = NULL; goto err; + } + + /* + * We must not fail now because the callback has prepended additional + * BIOs to the chain + */ ndef_aux->val = val; ndef_aux->it = it; @@ -91,11 +116,11 @@ BIO *BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it) ndef_aux->boundary = sarg.boundary; ndef_aux->out = out; - BIO_ctrl(asn_bio, BIO_C_SET_EX_ARG, 0, ndef_aux); - return sarg.ndef_bio; err: + /* BIO_pop() is NULL safe */ + (void)BIO_pop(pop_bio); BIO_free(asn_bio); OPENSSL_free(ndef_aux); return NULL; -- Gitee From 189231e16833ae96b600e7c4b61026dde9a8884f Mon Sep 17 00:00:00 2001 From: wanghao-free Date: Mon, 20 Feb 2023 00:26:00 -0800 Subject: [PATCH 2/2] fix CVE-2023-0286: Fix GENERAL_NAME_cmp for x400Address (1.1.1) Signed-off-by: wanghao-free --- CHANGES | 119 +++++++++++++++++++++++++++++++++++++++ crypto/x509v3/v3_genn.c | 2 +- include/openssl/x509v3.h | 2 +- 3 files changed, 121 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index f4230aaac0..cf331a648e 100755 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,125 @@ For a full list of changes, see the git commit log; for example, https://github.com/openssl/openssl/commits/ and pick the appropriate release branch. + + Changes between 1.1.1s and 1.1.1t [xx XXX xxxx] + + *) Fixed a type confusion vulnerability relating to X.400 address processing + inside an X.509 GeneralName. X.400 addresses were parsed as an ASN1_STRING + but subsequently interpreted by GENERAL_NAME_cmp as an ASN1_TYPE. This + vulnerability may allow an attacker who can provide a certificate chain and + CRL (neither of which need have a valid signature) to pass arbitrary + pointers to a memcmp call, creating a possible read primitive, subject to + some constraints. Refer to the advisory for more information. Thanks to + David Benjamin for discovering this issue. (CVE-2023-0286) + + This issue has been fixed by changing the public header file definition of + GENERAL_NAME so that x400Address reflects the implementation. It was not + possible for any existing application to successfully use the existing + definition; however, if any application references the x400Address field + (e.g. in dead code), note that the type of this field has changed. There is + no ABI change. + + [Hugo Landau] + + Changes between 1.1.1r and 1.1.1s [1 Nov 2022] + + *) Fixed a regression introduced in 1.1.1r version not refreshing the + certificate data to be signed before signing the certificate. + + [Gibeom Gwon] + + Changes between 1.1.1q and 1.1.1r [11 Oct 2022] + + *) Fixed the linux-mips64 Configure target which was missing the + SIXTY_FOUR_BIT bn_ops flag. This was causing heap corruption on that + platform. + [Adam Joseph] + + *) Fixed a strict aliasing problem in bn_nist. Clang-14 optimisation was + causing incorrect results in some cases as a result. + [Paul Dale] + + *) Fixed SSL_pending() and SSL_has_pending() with DTLS which were failing to + report correct results in some cases + [Matt Caswell] + + *) Fixed a regression introduced in 1.1.1o for re-signing certificates with + different key sizes + [Todd Short] + + *) Added the loongarch64 target + [Shi Pujin] + + *) Fixed a DRBG seed propagation thread safety issue + [Bernd Edlinger] + + *) Fixed a memory leak in tls13_generate_secret + [Bernd Edlinger] + + *) Fixed reported performance degradation on aarch64. Restored the + implementation prior to commit 2621751 ("aes/asm/aesv8-armx.pl: avoid + 32-bit lane assignment in CTR mode") for 64bit targets only, since it is + reportedly 2-17% slower and the silicon errata only affects 32bit targets. + The new algorithm is still used for 32 bit targets. + [Bernd Edlinger] + + *) Added a missing header for memcmp that caused compilation failure on some + platforms + [Gregor Jasny] + + Changes between 1.1.1p and 1.1.1q [5 Jul 2022] + + *) AES OCB mode for 32-bit x86 platforms using the AES-NI assembly optimised + implementation would not encrypt the entirety of the data under some + circumstances. This could reveal sixteen bytes of data that was + preexisting in the memory that wasn't written. In the special case of + "in place" encryption, sixteen bytes of the plaintext would be revealed. + + Since OpenSSL does not support OCB based cipher suites for TLS and DTLS, + they are both unaffected. + (CVE-2022-2097) + [Alex Chernyakhovsky, David Benjamin, Alejandro Sedeño] + + Changes between 1.1.1o and 1.1.1p [21 Jun 2022] + + *) In addition to the c_rehash shell command injection identified in + CVE-2022-1292, further bugs where the c_rehash script does not + properly sanitise shell metacharacters to prevent command injection have been + fixed. + + When the CVE-2022-1292 was fixed it was not discovered that there + are other places in the script where the file names of certificates + being hashed were possibly passed to a command executed through the shell. + + This script is distributed by some operating systems in a manner where + it is automatically executed. On such operating systems, an attacker + could execute arbitrary commands with the privileges of the script. + + Use of the c_rehash script is considered obsolete and should be replaced + by the OpenSSL rehash command line tool. + (CVE-2022-2068) + [Daniel Fiala, Tomáš Mráz] + + *) When OpenSSL TLS client is connecting without any supported elliptic + curves and TLS-1.3 protocol is disabled the connection will no longer fail + if a ciphersuite that does not use a key exchange based on elliptic + curves can be negotiated. + [Tomáš Mráz] + + Changes between 1.1.1n and 1.1.1o [3 May 2022] + + *) Fixed a bug in the c_rehash script which was not properly sanitising shell + metacharacters to prevent command injection. This script is distributed + by some operating systems in a manner where it is automatically executed. + On such operating systems, an attacker could execute arbitrary commands + with the privileges of the script. + + Use of the c_rehash script is considered obsolete and should be replaced + by the OpenSSL rehash command line tool. + (CVE-2022-1292) + [Tomáš Mráz] + Changes between 1.1.1e and 1.1.1f [31 Mar 2020] diff --git a/crypto/x509v3/v3_genn.c b/crypto/x509v3/v3_genn.c index 23778e2506..12ce73374d 100755 --- a/crypto/x509v3/v3_genn.c +++ b/crypto/x509v3/v3_genn.c @@ -97,7 +97,7 @@ 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); + result = ASN1_STRING_cmp(a->d.x400Address, b->d.x400Address); break; case GEN_EDIPARTY: diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h index 6c6eca38a5..b80438d2ff 100755 --- a/include/openssl/x509v3.h +++ b/include/openssl/x509v3.h @@ -136,7 +136,7 @@ typedef struct GENERAL_NAME_st { OTHERNAME *otherName; /* otherName */ ASN1_IA5STRING *rfc822Name; ASN1_IA5STRING *dNSName; - ASN1_TYPE *x400Address; + ASN1_STRING *x400Address; X509_NAME *directoryName; EDIPARTYNAME *ediPartyName; ASN1_IA5STRING *uniformResourceIdentifier; -- Gitee