Skip to content

Commit

Permalink
Fix a memory leak in EC_GROUP_new_from_ecparameters
Browse files Browse the repository at this point in the history
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
    openssl#6 0x7fb618e7aa13 in asn1_string_to_bn crypto/asn1/a_int.c:503
    openssl#7 0x7fb618e7aa13 in ASN1_INTEGER_to_BN crypto/asn1/a_int.c:559
    openssl#8 0x7fb618fd8e79 in EC_GROUP_new_from_ecparameters crypto/ec/ec_asn1.c:814
    openssl#9 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    openssl#10 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    openssl#11 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    openssl#12 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    openssl#13 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    openssl#14 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    openssl#15 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    openssl#16 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    openssl#17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#20 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#21 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#22 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#23 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    openssl#24 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    openssl#25 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    openssl#26 0x402afb in testfile fuzz/test-corpus.c:182
    openssl#27 0x402656 in main fuzz/test-corpus.c:226
    openssl#28 0x7fb618551f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    openssl#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
    openssl#6 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    openssl#7 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    openssl#8 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    openssl#9 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    openssl#10 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    openssl#11 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    openssl#12 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    openssl#13 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#14 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#15 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#16 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#17 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#18 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#19 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    openssl#20 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    openssl#21 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    openssl#22 0x402afb in testfile fuzz/test-corpus.c:182
    openssl#23 0x402656 in main fuzz/test-corpus.c:226
    openssl#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
    openssl#6 0x7fb618fd98e8 in EC_GROUP_new_from_ecpkparameters crypto/ec/ec_asn1.c:935
    openssl#7 0x7fb618fd9aec in d2i_ECPKParameters crypto/ec/ec_asn1.c:966
    openssl#8 0x7fb618fdace9 in d2i_ECParameters crypto/ec/ec_asn1.c:1184
    openssl#9 0x7fb618fd1fc7 in eckey_type2param crypto/ec/ec_ameth.c:119
    openssl#10 0x7fb618fd57b4 in eckey_pub_decode crypto/ec/ec_ameth.c:165
    openssl#11 0x7fb6191a9c62 in x509_pubkey_decode crypto/x509/x_pubkey.c:124
    openssl#12 0x7fb6191a9e42 in pubkey_cb crypto/x509/x_pubkey.c:46
    openssl#13 0x7fb618eac032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    openssl#14 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#15 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#16 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#17 0x7fb618eacaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#18 0x7fb618ead288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#19 0x7fb618eab9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#20 0x7fb618eadd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    openssl#21 0x7fb618eade35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    openssl#22 0x40310c in FuzzerTestOneInput fuzz/x509.c:33
    openssl#23 0x402afb in testfile fuzz/test-corpus.c:182
    openssl#24 0x402656 in main fuzz/test-corpus.c:226
    openssl#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 <[email protected]>
Reviewed-by: Kurt Roeckx <[email protected]>
(Merged from openssl#18633)

(cherry picked from commit be50862)
  • Loading branch information
bernd-edlinger committed Jun 25, 2022
1 parent 413e0db commit cc7c127
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions crypto/ec/ec_asn1.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,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) {
ERR_raise(ERR_LIB_EC, ERR_R_ASN1_LIB);
goto err;
}
Expand All @@ -747,7 +747,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) {
ERR_raise(ERR_LIB_EC, ERR_R_ASN1_LIB);
goto err;
}
Expand Down

0 comments on commit cc7c127

Please sign in to comment.