Skip to content

Conversation

@ndossche
Copy link
Member

When certificate cert exists, but is not added to the store, it causes memory leaks. The error handling was already existing but the freeing only happened on the success case.
One could also ponder whether it is necessary to inform the user when adding a certificate failed or signal this in some way.

Part of the leak report:

Direct leak of 384 byte(s) in 1 object(s) allocated from:
    #0 0x7fdbf1f9e9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fdbf183a7c4 in CRYPTO_zalloc (/lib/x86_64-linux-gnu/libcrypto.so.3+0x2237c4) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #2 0x7fdbf16f9d13  (/lib/x86_64-linux-gnu/libcrypto.so.3+0xe2d13) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #3 0x7fdbf16f9e19 in ASN1_item_new_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0xe2e19) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #4 0x7fdbf19a59f9 in X509_new_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0x38e9f9) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #5 0x5575bcd295cb in php_openssl_pem_read_bio_x509 /work/php-src/ext/openssl/openssl_backend_v3.c:876
    #6 0x5575bcd2ef3d in php_openssl_load_stream_cafile /work/php-src/ext/openssl/xp_ssl.c:855
    #7 0x5575bcd2f4da in php_openssl_enable_peer_verification /work/php-src/ext/openssl/xp_ssl.c:912
    #8 0x5575bcd33104 in php_openssl_setup_crypto /work/php-src/ext/openssl/xp_ssl.c:1610
    #9 0x5575bcd39c18 in php_openssl_sockop_set_option /work/php-src/ext/openssl/xp_ssl.c:2512
    #10 0x5575bdb4c610 in _php_stream_set_option /work/php-src/main/streams/streams.c:1466
    #11 0x5575bdb5557d in php_stream_xport_crypto_setup /work/php-src/main/streams/transports.c:367
    #12 0x5575bcd39f11 in php_openssl_sockop_set_option /work/php-src/ext/openssl/xp_ssl.c:2540
    #13 0x5575bdb4c610 in _php_stream_set_option /work/php-src/main/streams/streams.c:1466
    #14 0x5575bdb54655 in php_stream_xport_connect /work/php-src/main/streams/transports.c:248
    #15 0x5575bdb5365d in _php_stream_xport_create /work/php-src/main/streams/transports.c:145
    #16 0x5575bd8d30b1 in php_stream_url_wrap_http_ex /work/php-src/ext/standard/http_fopen_wrapper.c:490
    #17 0x5575bd8d857e in php_stream_url_wrap_http /work/php-src/ext/standard/http_fopen_wrapper.c:1204
    #18 0x5575bdb5073d in _php_stream_open_wrapper_ex /work/php-src/main/streams/streams.c:2270
    #19 0x5575bd878fa6 in zif_file_get_contents /work/php-src/ext/standard/file.c:409
    #20 0x5575bd5bfe39 in zif_phar_file_get_contents /work/php-src/ext/phar/func_interceptors.c:226
    #21 0x5575bdab7ed2 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
    #22 0x5575bdde024a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2154
    #23 0x5575bdf40995 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116519
    #24 0x5575bdf558b0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
    #25 0x5575be0ba0ab in zend_execute_script /work/php-src/Zend/zend.c:1980
    #26 0x5575bdaec8bb in php_execute_script_ex /work/php-src/main/main.c:2645
    #27 0x5575bdaecccb in php_execute_script /work/php-src/main/main.c:2685
    #28 0x5575be0bfc16 in do_cli /work/php-src/sapi/cli/php_cli.c:951
    #29 0x5575be0c21e3 in main /work/php-src/sapi/cli/php_cli.c:1362

... etc ...

This was found by a hybrid static-dynamic analyser that looks for inconsistent handling of error checks in bindings.

When certificate `cert` exists, but is not added to the store, it causes
memory leaks. The error handling was already existing but the freeing
only happened on the success case.
One could also ponder whether it is necessary to inform the user when
adding a certificate failed or signal this in some way.

Part of the leak report:
```
Direct leak of 384 byte(s) in 1 object(s) allocated from:
    #0 0x7fdbf1f9e9c7 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x7fdbf183a7c4 in CRYPTO_zalloc (/lib/x86_64-linux-gnu/libcrypto.so.3+0x2237c4) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #2 0x7fdbf16f9d13  (/lib/x86_64-linux-gnu/libcrypto.so.3+0xe2d13) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #3 0x7fdbf16f9e19 in ASN1_item_new_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0xe2e19) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #4 0x7fdbf19a59f9 in X509_new_ex (/lib/x86_64-linux-gnu/libcrypto.so.3+0x38e9f9) (BuildId: 0698e1ff610cb3c6993dccbd82c1281b1b4c5ade)
    #5 0x5575bcd295cb in php_openssl_pem_read_bio_x509 /work/php-src/ext/openssl/openssl_backend_v3.c:876
    #6 0x5575bcd2ef3d in php_openssl_load_stream_cafile /work/php-src/ext/openssl/xp_ssl.c:855
    #7 0x5575bcd2f4da in php_openssl_enable_peer_verification /work/php-src/ext/openssl/xp_ssl.c:912
    #8 0x5575bcd33104 in php_openssl_setup_crypto /work/php-src/ext/openssl/xp_ssl.c:1610
    #9 0x5575bcd39c18 in php_openssl_sockop_set_option /work/php-src/ext/openssl/xp_ssl.c:2512
    #10 0x5575bdb4c610 in _php_stream_set_option /work/php-src/main/streams/streams.c:1466
    #11 0x5575bdb5557d in php_stream_xport_crypto_setup /work/php-src/main/streams/transports.c:367
    #12 0x5575bcd39f11 in php_openssl_sockop_set_option /work/php-src/ext/openssl/xp_ssl.c:2540
    #13 0x5575bdb4c610 in _php_stream_set_option /work/php-src/main/streams/streams.c:1466
    #14 0x5575bdb54655 in php_stream_xport_connect /work/php-src/main/streams/transports.c:248
    #15 0x5575bdb5365d in _php_stream_xport_create /work/php-src/main/streams/transports.c:145
    #16 0x5575bd8d30b1 in php_stream_url_wrap_http_ex /work/php-src/ext/standard/http_fopen_wrapper.c:490
    #17 0x5575bd8d857e in php_stream_url_wrap_http /work/php-src/ext/standard/http_fopen_wrapper.c:1204
    #18 0x5575bdb5073d in _php_stream_open_wrapper_ex /work/php-src/main/streams/streams.c:2270
    #19 0x5575bd878fa6 in zif_file_get_contents /work/php-src/ext/standard/file.c:409
    #20 0x5575bd5bfe39 in zif_phar_file_get_contents /work/php-src/ext/phar/func_interceptors.c:226
    #21 0x5575bdab7ed2 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
    #22 0x5575bdde024a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2154
    #23 0x5575bdf40995 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116519
    #24 0x5575bdf558b0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
    #25 0x5575be0ba0ab in zend_execute_script /work/php-src/Zend/zend.c:1980
    #26 0x5575bdaec8bb in php_execute_script_ex /work/php-src/main/main.c:2645
    #27 0x5575bdaecccb in php_execute_script /work/php-src/main/main.c:2685
    #28 0x5575be0bfc16 in do_cli /work/php-src/sapi/cli/php_cli.c:951
    #29 0x5575be0c21e3 in main /work/php-src/sapi/cli/php_cli.c:1362

... etc ...
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant