Skip to content

Conversation

@ndossche
Copy link
Member

Leak report:

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f97cf4cb340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x7f97cef66106 in BN_new bn/bn_lib.c:75
    #2 0x7f97cef6006c in bn_bin2bn_cbs bn/bn_convert.c:151
    #3 0x7f97cef60853 in BN_bin2bn bn/bn_convert.c:206
    #4 0x56229112465b in php_openssl_pkey_init_dh_data /work/php-src/ext/openssl/openssl_backend_v1.c:208
    #5 0x5622911248be in php_openssl_pkey_init_dh /work/php-src/ext/openssl/openssl_backend_v1.c:246
    #6 0x5622910fe1d7 in zif_openssl_pkey_new /work/php-src/ext/openssl/openssl.c:2051
    #7 0x562291eb44e5 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
    #8 0x5622921dc85a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2154
    #9 0x56229233cfa5 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116519
    #10 0x562292351ec0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
    #11 0x5622924b60cc in zend_execute_script /work/php-src/Zend/zend.c:1980
    #12 0x562291ee8ecb in php_execute_script_ex /work/php-src/main/main.c:2645
    #13 0x562291ee92db in php_execute_script /work/php-src/main/main.c:2685
    #14 0x5622924bbc37 in do_cli /work/php-src/sapi/cli/php_cli.c:951
    #15 0x5622924be204 in main /work/php-src/sapi/cli/php_cli.c:1362
    #16 0x7f97ceb301c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #17 0x7f97ceb3028a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #18 0x562291009db4 in _start (/work/php-src/build-dbg-asan/sapi/cli/php+0x609db4) (BuildId: 5cc444a6a9fc1a486ea698e72366c16bd5472605)

... etc ...

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

Leak report:
```
Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f97cf4cb340 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x7f97cef66106 in BN_new bn/bn_lib.c:75
    #2 0x7f97cef6006c in bn_bin2bn_cbs bn/bn_convert.c:151
    #3 0x7f97cef60853 in BN_bin2bn bn/bn_convert.c:206
    #4 0x56229112465b in php_openssl_pkey_init_dh_data /work/php-src/ext/openssl/openssl_backend_v1.c:208
    #5 0x5622911248be in php_openssl_pkey_init_dh /work/php-src/ext/openssl/openssl_backend_v1.c:246
    #6 0x5622910fe1d7 in zif_openssl_pkey_new /work/php-src/ext/openssl/openssl.c:2051
    #7 0x562291eb44e5 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
    #8 0x5622921dc85a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2154
    #9 0x56229233cfa5 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116519
    #10 0x562292351ec0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
    #11 0x5622924b60cc in zend_execute_script /work/php-src/Zend/zend.c:1980
    #12 0x562291ee8ecb in php_execute_script_ex /work/php-src/main/main.c:2645
    #13 0x562291ee92db in php_execute_script /work/php-src/main/main.c:2685
    #14 0x5622924bbc37 in do_cli /work/php-src/sapi/cli/php_cli.c:951
    #15 0x5622924be204 in main /work/php-src/sapi/cli/php_cli.c:1362
    #16 0x7f97ceb301c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #17 0x7f97ceb3028a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #18 0x562291009db4 in _start (/work/php-src/build-dbg-asan/sapi/cli/php+0x609db4) (BuildId: 5cc444a6a9fc1a486ea698e72366c16bd5472605)

... etc ...
```
if (priv_key) {
pub_key = php_openssl_dh_pub_from_priv(priv_key, g, p);
if (pub_key == NULL) {
BN_free(p);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks fine here; while not being an expert of openssl, what do you think of the code path from line 4259 ? e.g. should g be freed is not null ?

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.

2 participants