From dc864e36544e7fb5a32516b7d178a6b4a86b70c6 Mon Sep 17 00:00:00 2001 From: tb Date: Sat, 11 Mar 2023 15:50:13 +0000 Subject: [PATCH] Fix double free after BIO_new_NDEF() Once the asn_bio is prepended to the out chain, and before the asn1_cb() has done its thing, asn_bio needs to be popped off again on error. Failing to do this can cause write after frees or double frees when the out BIO is used after the function returned. Based on a very complicated diff by Matt Caswell and Viktor Dukhovni. This was part of the fixes in OpenSSL 1.1.1t. ok jsing --- lib/libcrypto/asn1/bio_ndef.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/libcrypto/asn1/bio_ndef.c b/lib/libcrypto/asn1/bio_ndef.c index 6266d68bbd7..7ef08764aab 100644 --- a/lib/libcrypto/asn1/bio_ndef.c +++ b/lib/libcrypto/asn1/bio_ndef.c @@ -1,4 +1,4 @@ -/* $OpenBSD: bio_ndef.c,v 1.12 2023/03/06 19:10:14 tb Exp $ */ +/* $OpenBSD: bio_ndef.c,v 1.13 2023/03/11 15:50:13 tb Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project. */ @@ -101,7 +101,7 @@ BIO * BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it) { NDEF_SUPPORT *ndef_aux = NULL; - BIO *asn_bio = NULL; + BIO *asn_bio = NULL, *pop_bio = NULL; const ASN1_AUX *aux = it->funcs; ASN1_STREAM_ARG sarg; @@ -112,12 +112,12 @@ BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it) ndef_aux = malloc(sizeof(NDEF_SUPPORT)); asn_bio = BIO_new(BIO_f_asn1()); - /* ASN1 bio needs to be next to output BIO */ - - out = BIO_push(asn_bio, out); + if (!ndef_aux || !asn_bio) + goto err; - if (!ndef_aux || !asn_bio || !out) + if ((out = BIO_push(asn_bio, 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); @@ -144,6 +144,7 @@ BIO_new_NDEF(BIO *out, ASN1_VALUE *val, const ASN1_ITEM *it) return sarg.ndef_bio; err: + BIO_pop(pop_bio); BIO_free(asn_bio); free(ndef_aux); return NULL; -- 2.20.1