107 lines
3.6 KiB
Diff
107 lines
3.6 KiB
Diff
From c6ca9c4b78a6e88f3517e167ff6d498623bf296a Mon Sep 17 00:00:00 2001
|
|
From: starlet-dx <15929766099@163.com>
|
|
Date: Wed, 22 Feb 2023 19:06:45 +0800
|
|
Subject: [PATCH 1/1] 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 <pauli@openssl.org>
|
|
Reviewed-by: Tomas Mraz <tomas@openssl.org>
|
|
---
|
|
deps/openssl/openssl/crypto/asn1/bio_ndef.c | 39 +++++++++++++++++----
|
|
1 file changed, 32 insertions(+), 7 deletions(-)
|
|
|
|
diff --git a/deps/openssl/openssl/crypto/asn1/bio_ndef.c b/deps/openssl/openssl/crypto/asn1/bio_ndef.c
|
|
index 760e4846..f8d4b1b9 100644
|
|
--- a/deps/openssl/openssl/crypto/asn1/bio_ndef.c
|
|
+++ b/deps/openssl/openssl/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;
|
|
--
|
|
2.30.0
|
|
|