Fix a memory leak in crl_set_issuers

bernd-edlinger · bernd-edlinger · commit e9007e09792e · 2022-05-25T13:04:09.000+02:00
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 rogii-com#1 0x402fc4 in my_realloc fuzz/test-corpus.c:129 rogii-com#2 0x7fd5d8893c49 in sk_reserve crypto/stack/stack.c:198 rogii-com#3 0x7fd5d8893c49 in OPENSSL_sk_insert crypto/stack/stack.c:242 rogii-com#4 0x7fd5d88d6d7f in sk_GENERAL_NAMES_push include/openssl/x509v3.h:168 rogii-com#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 rogii-com#1 0x7fd5d87c2430 in CRYPTO_zalloc crypto/mem.c:230 rogii-com#2 0x7fd5d889501f in OPENSSL_sk_new_reserve crypto/stack/stack.c:209 rogii-com#3 0x7fd5d85dcbc3 in sk_ASN1_VALUE_new_null include/openssl/asn1t.h:928 rogii-com#4 0x7fd5d85dcbc3 in asn1_template_noexp_d2i crypto/asn1/tasn_dec.c:577 rogii-com#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 <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from openssl#18391)
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
@@ -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;
 
