Skip to content

Fix memory leak on error path in openssl_open() - #21054

Closed
ndossche wants to merge 1 commit into
php:PHP-8.4from
ndossche:clesss-22
Closed

Fix memory leak on error path in openssl_open()#21054
ndossche wants to merge 1 commit into
php:PHP-8.4from
ndossche:clesss-22

Conversation

@ndossche

Copy link
Copy Markdown
Member

pkey must be released after it was allocated on the error paths. Otherwise we get leaks like this:

Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7ff8d76a1340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x7ff8d7189136 in EVP_PKEY_new evp/p_lib.c:385
#2 0x7ff8d71178e4 in d2i_PrivateKey asn1/a_pkey.c:80
#3 0x7ff8d719ed07 in PEM_read_bio_PrivateKey pem/pem_pkey.c:135
#4 0x555c54726e80 in php_openssl_pem_read_bio_private_key /work/php-src/ext/openssl/openssl_backend_v1.c:738
#5 0x555c5471ee77 in php_openssl_pkey_from_zval /work/php-src/ext/openssl/openssl_backend_common.c:1297
#6 0x555c54712e3f in zif_openssl_open /work/php-src/ext/openssl/openssl.c:4331
#7 0x555c554b44e5 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
#8 0x555c557dba0b in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2024
#9 0x555c5593cf57 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116514
#10 0x555c55951ec0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
#11 0x555c55ab60cc in zend_execute_script /work/php-src/Zend/zend.c:1980
#12 0x555c554e8ecb in php_execute_script_ex /work/php-src/main/main.c:2645
#13 0x555c554e92db in php_execute_script /work/php-src/main/main.c:2685
#14 0x555c55abbc37 in do_cli /work/php-src/sapi/cli/php_cli.c:951
#15 0x555c55abe204 in main /work/php-src/sapi/cli/php_cli.c:1362
#16 0x7ff8d6d061c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#17 0x7ff8d6d0628a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#18 0x555c54609db4 in _start (/work/php-src/build-dbg-asan/sapi/cli/php+0x609db4) (BuildId: 5cc444a6a9fc1a486ea698e72366c16bd5472605)

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

`pkey` must be released after it was allocated on the error paths.
Otherwise we get leaks like this:
```
Direct leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7ff8d76a1340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
#1 0x7ff8d7189136 in EVP_PKEY_new evp/p_lib.c:385
#2 0x7ff8d71178e4 in d2i_PrivateKey asn1/a_pkey.c:80
#3 0x7ff8d719ed07 in PEM_read_bio_PrivateKey pem/pem_pkey.c:135
#4 0x555c54726e80 in php_openssl_pem_read_bio_private_key /work/php-src/ext/openssl/openssl_backend_v1.c:738
#5 0x555c5471ee77 in php_openssl_pkey_from_zval /work/php-src/ext/openssl/openssl_backend_common.c:1297
#6 0x555c54712e3f in zif_openssl_open /work/php-src/ext/openssl/openssl.c:4331
#7 0x555c554b44e5 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
#8 0x555c557dba0b in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2024
#9 0x555c5593cf57 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116514
#10 0x555c55951ec0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
#11 0x555c55ab60cc in zend_execute_script /work/php-src/Zend/zend.c:1980
#12 0x555c554e8ecb in php_execute_script_ex /work/php-src/main/main.c:2645
#13 0x555c554e92db in php_execute_script /work/php-src/main/main.c:2685
#14 0x555c55abbc37 in do_cli /work/php-src/sapi/cli/php_cli.c:951
#15 0x555c55abe204 in main /work/php-src/sapi/cli/php_cli.c:1362
#16 0x7ff8d6d061c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#17 0x7ff8d6d0628a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#18 0x555c54609db4 in _start (/work/php-src/build-dbg-asan/sapi/cli/php+0x609db4) (BuildId: 5cc444a6a9fc1a486ea698e72366c16bd5472605)
```
@ndossche
ndossche requested a review from bukka as a code ownerJanuary 27, 2026 21:45
@ndossche
ndossche changed the base branch from master to PHP-8.4January 27, 2026 21:45
Sign up for freeto 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.

3 participants

@ndossche@bukka@iluuu1994