Skip to content

Commit

Permalink
Fix a memory leak in crl_set_issuers
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=1653520461 ../util/shlib_wrap.sh ./cms-test ./corpora/cms/3eff1d2f1232bd66d5635db2c3f9e7f23830dfd1
log file: cms-3eff1d2f1232bd66d5635db2c3f9e7f23830dfd1-32454-test.out
ERROR_INJECT=1653520461
    #0 0x7fd5d8b8eeba in __sanitizer_print_stack_trace ../../../../gcc-trunk/libsanitizer/asan/asan_stack.cpp:87
    #1 0x402fc4 in my_realloc fuzz/test-corpus.c:129
    #2 0x7fd5d8893c49 in sk_reserve crypto/stack/stack.c:198
    #3 0x7fd5d8893c49 in OPENSSL_sk_insert crypto/stack/stack.c:242
    #4 0x7fd5d88d6d7f in sk_GENERAL_NAMES_push include/openssl/x509v3.h:168
    #5 0x7fd5d88d6d7f in crl_set_issuers crypto/x509/x_crl.c:111
    openssl#6 0x7fd5d88d6d7f in crl_cb crypto/x509/x_crl.c:246
    openssl#7 0x7fd5d85dc032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    openssl#8 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#9 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#10 0x7fd5d85db2b5 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:259
    openssl#11 0x7fd5d85dc813 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:611
    openssl#12 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#13 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#14 0x7fd5d85dca28 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:633
    openssl#15 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#16 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#17 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#18 0x7fd5d85dd7d3 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:494
    openssl#19 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#20 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    openssl#21 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    openssl#22 0x7fd5d85a77e0 in ASN1_item_d2i_bio crypto/asn1/a_d2i_fp.c:69
    openssl#23 0x402845 in FuzzerTestOneInput fuzz/cms.c:43
    openssl#24 0x402bbb in testfile fuzz/test-corpus.c:182
    openssl#25 0x402626 in main fuzz/test-corpus.c:226
    openssl#26 0x7fd5d7c81f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
    openssl#27 0x402706  (/home/ed/OPC/openssl/fuzz/cms-test+0x402706)

=================================================================
==29625==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7fd5d8b8309f in __interceptor_malloc ../../../../gcc-trunk/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fd5d87c2430 in CRYPTO_zalloc crypto/mem.c:230
    #2 0x7fd5d889501f in OPENSSL_sk_new_reserve crypto/stack/stack.c:209
    #3 0x7fd5d85dcbc3 in sk_ASN1_VALUE_new_null include/openssl/asn1t.h:928
    #4 0x7fd5d85dcbc3 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:577
    #5 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#6 0x7fd5d85db104 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:178
    openssl#7 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    openssl#8 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    openssl#9 0x7fd5d88f86d9 in X509V3_EXT_d2i crypto/x509v3/v3_lib.c:142
    openssl#10 0x7fd5d88d6d3c in crl_set_issuers crypto/x509/x_crl.c:97
    openssl#11 0x7fd5d88d6d3c in crl_cb crypto/x509/x_crl.c:246
    openssl#12 0x7fd5d85dc032 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:432
    openssl#13 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#14 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#15 0x7fd5d85db2b5 in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:259
    openssl#16 0x7fd5d85dc813 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:611
    openssl#17 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#18 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#19 0x7fd5d85dca28 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:633
    openssl#20 0x7fd5d85dd288 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:518
    openssl#21 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#22 0x7fd5d85dcaf5 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:643
    openssl#23 0x7fd5d85dd7d3 in asn1_template_ex_d2i crypto/asn1/tasn_dec.c:494
    openssl#24 0x7fd5d85db9ce in asn1_item_embed_d2i crypto/asn1/tasn_dec.c:382
    openssl#25 0x7fd5d85ddd1f in ASN1_item_ex_d2i crypto/asn1/tasn_dec.c:124
    openssl#26 0x7fd5d85dde35 in ASN1_item_d2i crypto/asn1/tasn_dec.c:114
    openssl#27 0x7fd5d85a77e0 in ASN1_item_d2i_bio crypto/asn1/a_d2i_fp.c:69
    openssl#28 0x402845 in FuzzerTestOneInput fuzz/cms.c:43
    openssl#29 0x402bbb in testfile fuzz/test-corpus.c:182
    openssl#30 0x402626 in main fuzz/test-corpus.c:226
    openssl#31 0x7fd5d7c81f44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)

SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s).

Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#18391)
  • Loading branch information
bernd-edlinger committed May 25, 2022
1 parent d1460af commit e9007e0
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions crypto/x509/x_crl.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,17 @@ static int crl_set_issuers(X509_CRL *crl)

if (gtmp) {
gens = gtmp;
if (!crl->issuers) {
if (crl->issuers == NULL) {
crl->issuers = sk_GENERAL_NAMES_new_null();
if (!crl->issuers)
if (crl->issuers == NULL) {
GENERAL_NAMES_free(gtmp);
return 0;
}
}
if (!sk_GENERAL_NAMES_push(crl->issuers, gtmp))
if (!sk_GENERAL_NAMES_push(crl->issuers, gtmp)) {
GENERAL_NAMES_free(gtmp);
return 0;
}
}
rev->issuer = gens;

Expand Down

0 comments on commit e9007e0

Please sign in to comment.