Fix a memory leak in EC_GROUP_new_from_ecparameters
authorBernd Edlinger <bernd.edlinger@hotmail.de>
Wed, 22 Jun 2022 15:05:55 +0000 (17:05 +0200)
committerBernd Edlinger <bernd.edlinger@hotmail.de>
Sat, 25 Jun 2022 05:22:49 +0000 (07:22 +0200)
This can be reproduced with my error injection patch.

The test vector has been validated on the 1.1.1 branch
but the issue is of course identical in all branches.

$ ERROR_INJECT=1656112173 ../util/shlib_wrap.sh ./x509-test ./corpora/x509/fe543a8d7e09109a9a08114323eefec802ad79e2
    #0 0x7fb61945eeba in __sanitizer_print_stack_trace ../../../../gcc-trunk/libsanitizer/asan/asan_stack.cpp:87
    #1 0x402f84 in my_malloc fuzz/test-corpus.c:114
    #2 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #3 0x7fb618ef7561 in bn_expand_internal crypto/bn/bn_lib.c:280
    #4 0x7fb618ef7561 in bn_expand2 crypto/bn/bn_lib.c:304
    #5 0x7fb618ef819d in BN_bin2bn crypto/bn/bn_lib.c:454
    #6 0x7fb618e7aa13 in asn1_string_to_bn crypto/asn1/a_int.c:503
    #7 0x7fb618e7aa13 in ASN1_INTEGER_to_BN crypto/asn1/a_int.c:559
    #8 0x7fb618fd8e79 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:814
    #9 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #10 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #11 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #12 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #13 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #14 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #15 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #16 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #21 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #22 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #23 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #24 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #25 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #26 0x402afb in testfile fuzz/test-corpus.c:182
    #27 0x402656 in main fuzz/test-corpus.c:226
    #28 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    #29 0x402756  (/home/ed/OPC/openssl/fuzz/x509-test+0x402756)

=================================================================
==12221==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7fb61945309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fb618ef5f11 in BN_new crypto/bn/bn_lib.c:246
    #3 0x7fb618ef82f4 in BN_bin2bn crypto/bn/bn_lib.c:440
    #4 0x7fb618fd8933 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:618
    #5 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #6 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #7 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #8 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #9 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #10 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #11 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #12 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #13 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #14 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #15 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #16 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #17 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #18 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #19 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #20 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #21 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #22 0x402afb in testfile fuzz/test-corpus.c:182
    #23 0x402656 in main fuzz/test-corpus.c:226
    #24 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

Indirect leak of 56 byte(s) in 1 object(s) allocated from:
    #0 0x7fb61945309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fb619092430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fb618ef7561 in bn_expand_internal crypto/bn/bn_lib.c:280
    #3 0x7fb618ef7561 in bn_expand2 crypto/bn/bn_lib.c:304
    #4 0x7fb618ef819d in BN_bin2bn crypto/bn/bn_lib.c:454
    #5 0x7fb618fd8933 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:618
    #6 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    #7 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    #8 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    #9 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    #10 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    #11 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    #12 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    #13 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    #14 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #15 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #16 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    #18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    #19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    #20 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    #21 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    #22 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    #23 0x402afb in testfile fuzz/test-corpus.c:182
    #24 0x402656 in main fuzz/test-corpus.c:226
    #25 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

SUMMARY: AddressSanitizer: 80 byte(s) leaked in 2 allocation(s).

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/18632)

crypto/ec/ec_asn1.c

index 34de7b2aabf704ed4ab98fe9940f1b6266ddd8d2..1acbbde3d37b6f64988858d6d71732557196264c 100644 (file)
@@ -794,7 +794,7 @@ EC_GROUP *EC_GROUP_new_from_ecparameters(const ECPARAMETERS *params)
     }
 
     /* extract the order */
-    if ((a = ASN1_INTEGER_to_BN(params->order, a)) == NULL) {
+    if (ASN1_INTEGER_to_BN(params->order, a) == NULL) {
         ECerr(EC_F_EC_GROUP_NEW_FROM_ECPARAMETERS, ERR_R_ASN1_LIB);
         goto err;
     }
@@ -811,7 +811,7 @@ EC_GROUP *EC_GROUP_new_from_ecparameters(const ECPARAMETERS *params)
     if (params->cofactor == NULL) {
         BN_free(b);
         b = NULL;
-    } else if ((b = ASN1_INTEGER_to_BN(params->cofactor, b)) == NULL) {
+    } else if (ASN1_INTEGER_to_BN(params->cofactor, b) == NULL) {
         ECerr(EC_F_EC_GROUP_NEW_FROM_ECPARAMETERS, ERR_R_ASN1_LIB);
         goto err;
     }