From 73ebaac827180bb51ccf807673758d7d06d5db21 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Fri, 22 Dec 2023 19:28:38 +0100 Subject: [PATCH] Fix error handling in ASN1_mbstring_ncopy Sometimes the error handling returns an ASN1_STRING object in *out although that was not passed in by the caller, and sometimes the error handling deletes the ASN1_STRING but forgets to clear the *out parameter. Therefore the caller has no chance to know, if the leaked object in *out shall be deleted or not. This may cause a use-after-free error e.g. in asn1_str2type: ==63312==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000073280 at pc 0x7f2652e93b08 bp 0x7ffe0e1951c0 sp 0x7ffe0e1951b0 READ of size 8 at 0x603000073280 thread T0 #0 0x7f2652e93b07 in asn1_string_embed_free crypto/asn1/asn1_lib.c:354 #1 0x7f2652eb521a in asn1_primitive_free crypto/asn1/tasn_fre.c:204 #2 0x7f2652eb50a9 in asn1_primitive_free crypto/asn1/tasn_fre.c:199 #3 0x7f2652eb5b67 in ASN1_item_free crypto/asn1/tasn_fre.c:20 #4 0x7f2652e8e13b in asn1_str2type crypto/asn1/asn1_gen.c:740 #5 0x7f2652e8e13b in generate_v3 crypto/asn1/asn1_gen.c:137 #6 0x7f2652e9166c in ASN1_generate_v3 crypto/asn1/asn1_gen.c:92 #7 0x7f2653307b9b in do_othername crypto/x509v3/v3_alt.c:577 #8 0x7f2653307b9b in a2i_GENERAL_NAME crypto/x509v3/v3_alt.c:492 #9 0x7f26533087c2 in v2i_subject_alt crypto/x509v3/v3_alt.c:327 #10 0x7f26533107fc in do_ext_nconf crypto/x509v3/v3_conf.c:100 #11 0x7f2653310f33 in X509V3_EXT_nconf crypto/x509v3/v3_conf.c:45 #12 0x7f2653311426 in X509V3_EXT_add_nconf_sk crypto/x509v3/v3_conf.c:312 #13 0x7f265331170c in X509V3_EXT_REQ_add_nconf crypto/x509v3/v3_conf.c:360 #14 0x564ed19d5f25 in req_main apps/req.c:806 #15 0x564ed19b8de0 in do_cmd apps/openssl.c:564 #16 0x564ed1985165 in main apps/openssl.c:183 #17 0x7f2651c4a082 in __libc_start_main ../csu/libc-start.c:308 #18 0x564ed1985acd in _start (/home/ed/OPCToolboxV5/Source/Core/OpenSSL/openssl/apps/openssl+0x139acd) 0x603000073280 is located 16 bytes inside of 24-byte region [0x603000073270,0x603000073288) freed by thread T0 here: #0 0x7f265413440f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122 #1 0x7f265315a429 in CRYPTO_free crypto/mem.c:311 #2 0x7f265315a429 in CRYPTO_free crypto/mem.c:300 #3 0x7f2652e757b9 in ASN1_mbstring_ncopy crypto/asn1/a_mbstr.c:191 #4 0x7f2652e75ec5 in ASN1_mbstring_copy crypto/asn1/a_mbstr.c:38 #5 0x7f2652e8e227 in asn1_str2type crypto/asn1/asn1_gen.c:681 #6 0x7f2652e8e227 in generate_v3 crypto/asn1/asn1_gen.c:137 #7 0x7f2652e9166c in ASN1_generate_v3 crypto/asn1/asn1_gen.c:92 #8 0x7f2653307b9b in do_othername crypto/x509v3/v3_alt.c:577 #9 0x7f2653307b9b in a2i_GENERAL_NAME crypto/x509v3/v3_alt.c:492 #10 0x7f26533087c2 in v2i_subject_alt crypto/x509v3/v3_alt.c:327 #11 0x7f26533107fc in do_ext_nconf crypto/x509v3/v3_conf.c:100 #12 0x7f2653310f33 in X509V3_EXT_nconf crypto/x509v3/v3_conf.c:45 #13 0x7f2653311426 in X509V3_EXT_add_nconf_sk crypto/x509v3/v3_conf.c:312 #14 0x7f265331170c in X509V3_EXT_REQ_add_nconf crypto/x509v3/v3_conf.c:360 #15 0x564ed19d5f25 in req_main apps/req.c:806 #16 0x564ed19b8de0 in do_cmd apps/openssl.c:564 #17 0x564ed1985165 in main apps/openssl.c:183 #18 0x7f2651c4a082 in __libc_start_main ../csu/libc-start.c:308 previously allocated by thread T0 here: #0 0x7f2654134808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144 #1 0x7f265315a4fd in CRYPTO_malloc crypto/mem.c:221 #2 0x7f265315a4fd in CRYPTO_malloc crypto/mem.c:198 #3 0x7f265315a945 in CRYPTO_zalloc crypto/mem.c:236 #4 0x7f2652e939a4 in ASN1_STRING_type_new crypto/asn1/asn1_lib.c:341 #5 0x7f2652e74e51 in ASN1_mbstring_ncopy crypto/asn1/a_mbstr.c:150 #6 0x7f2652e75ec5 in ASN1_mbstring_copy crypto/asn1/a_mbstr.c:38 #7 0x7f2652e8e227 in asn1_str2type crypto/asn1/asn1_gen.c:681 #8 0x7f2652e8e227 in generate_v3 crypto/asn1/asn1_gen.c:137 #9 0x7f2652e9166c in ASN1_generate_v3 crypto/asn1/asn1_gen.c:92 #10 0x7f2653307b9b in do_othername crypto/x509v3/v3_alt.c:577 #11 0x7f2653307b9b in a2i_GENERAL_NAME crypto/x509v3/v3_alt.c:492 #12 0x7f26533087c2 in v2i_subject_alt crypto/x509v3/v3_alt.c:327 #13 0x7f26533107fc in do_ext_nconf crypto/x509v3/v3_conf.c:100 #14 0x7f2653310f33 in X509V3_EXT_nconf crypto/x509v3/v3_conf.c:45 #15 0x7f2653311426 in X509V3_EXT_add_nconf_sk crypto/x509v3/v3_conf.c:312 #16 0x7f265331170c in X509V3_EXT_REQ_add_nconf crypto/x509v3/v3_conf.c:360 #17 0x564ed19d5f25 in req_main apps/req.c:806 #18 0x564ed19b8de0 in do_cmd apps/openssl.c:564 #19 0x564ed1985165 in main apps/openssl.c:183 #20 0x7f2651c4a082 in __libc_start_main ../csu/libc-start.c:308 Reviewed-by: Paul Yang Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/23138) --- crypto/asn1/a_mbstr.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crypto/asn1/a_mbstr.c b/crypto/asn1/a_mbstr.c index 7d80798655..c8170e1626 100644 --- a/crypto/asn1/a_mbstr.c +++ b/crypto/asn1/a_mbstr.c @@ -139,7 +139,7 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len, if (*out) { free_out = 0; dest = *out; - ASN1_STRING_set0(dest, NULL, 0); + ASN1_STRING_set0(dest, NULL, 0); dest->type = str_type; } else { free_out = 1; @@ -153,6 +153,10 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len, /* If both the same type just copy across */ if (inform == outform) { if (!ASN1_STRING_set(dest, in, len)) { + if (free_out) { + ASN1_STRING_free(dest); + *out = NULL; + } ERR_raise(ERR_LIB_ASN1, ERR_R_ASN1_LIB); return -1; } @@ -183,8 +187,10 @@ int ASN1_mbstring_ncopy(ASN1_STRING **out, const unsigned char *in, int len, break; } if ((p = OPENSSL_malloc(outlen + 1)) == NULL) { - if (free_out) + if (free_out) { ASN1_STRING_free(dest); + *out = NULL; + } return -1; } dest->length = outlen;