Skip to content

Conversation

@ndossche
Copy link
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 owner January 27, 2026 21:45
@ndossche ndossche changed the base branch from master to PHP-8.4 January 27, 2026 21:45
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