Fixing memory leak in openssl_pkcs12_read when zout initialisation fails

[JarneClauw]

=================================================================
==3182609==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x7fcdca2e6f2b in malloc (/lib64/libasan.so.8+0xe6f2b) (BuildId: 25975f766867e9e604dc5a71a8befeaed3301942)
    #1 0x7fcdc9b38bbd in CRYPTO_malloc (/lib64/libcrypto.so.3+0x138bbd) (BuildId: b0b5c707569ba97d977637f93a8851c5a6fb9f31)
    #2 0x7fcdc9b38ed4 in CRYPTO_zalloc (/lib64/libcrypto.so.3+0x138ed4) (BuildId: b0b5c707569ba97d977637f93a8851c5a6fb9f31)
    #3 0x7fcdc9ba78d9 in OPENSSL_sk_new_reserve (/lib64/libcrypto.so.3+0x1a78d9) (BuildId: b0b5c707569ba97d977637f93a8851c5a6fb9f31)
    #4 0x7fcdc9bd954f in ossl_x509_add_cert_new (/lib64/libcrypto.so.3+0x1d954f) (BuildId: b0b5c707569ba97d977637f93a8851c5a6fb9f31)
    #5 0x7fcdc9b7694f in PKCS12_parse (/lib64/libcrypto.so.3+0x17694f) (BuildId: b0b5c707569ba97d977637f93a8851c5a6fb9f31)
    #6 0x0000004f6310 in zif_openssl_pkcs12_read /home/jarne/ugent/mastersThesis/project/php/ext/openssl/openssl.c:2844
    #7 0x0000011b2b4b in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/jarne/ugent/mastersThesis/project/php/Zend/zend_vm_execute.h:1351
    #8 0x0000013221c4 in execute_ex /home/jarne/ugent/mastersThesis/project/php/Zend/zend_vm_execute.h:58907
    #9 0x0000013362af in zend_execute /home/jarne/ugent/mastersThesis/project/php/Zend/zend_vm_execute.h:64334
    #10 0x0000014da87c in zend_execute_script /home/jarne/ugent/mastersThesis/project/php/Zend/zend.c:1934
    #11 0x000000ec59b6 in php_execute_script_ex /home/jarne/ugent/mastersThesis/project/php/main/main.c:2577
    #12 0x000000ec6023 in php_execute_script /home/jarne/ugent/mastersThesis/project/php/main/main.c:2617
    #13 0x0000014e07c8 in do_cli /home/jarne/ugent/mastersThesis/project/php/sapi/cli/php_cli.c:935
    #14 0x0000014e2ac5 in main /home/jarne/ugent/mastersThesis/project/php/sapi/cli/php_cli.c:1310
    #15 0x7fcdc96965b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4) (BuildId: ff0267465bc3d76e21003b3bc5598fd5ee63e261)
    #16 0x7fcdc9696667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667) (BuildId: ff0267465bc3d76e21003b3bc5598fd5ee63e261)
    #17 0x000000405ef4 in _start (/home/jarne/ugent/mastersThesis/project/php/sapi/cli/php+0x405ef4) (BuildId: d652cea7d314fb3456c0cef256aa84673326780c)

Found by a static-dynamic analyzer looking for memory bugs in error-handling paths.

[ndossche]

Dit is vrij ingewikkeld, en er ontbreekt een test.
Een simpele test zou dit kunnen zijn:

--TEST--
Memory leak when array initialization in openssl_pkcs12_read() fails
--EXTENSIONS--
openssl
--FILE--
<?php
$pfx = __DIR__ . DIRECTORY_SEPARATOR . "bug74022.pfx";
$cert_store = file_get_contents($pfx);

class Typed {
    public string $foo = "bar";
}

$typed = new Typed;

try {
    openssl_pkcs12_read($cert_store, $typed->foo, "csos");
} catch (TypeError $e) {
    echo $e::class, ": ", $e->getMessage(), "\n";
}
?>
--EXPECT--
TypeError: Cannot assign array to reference held by property Typed::$foo of type string

Waarschijnlijk is het simpeler om een one-line fix te doen in het codeblock direct na de gefaalde check; en sk_X509_pop_free te gebruiken?

[ndossche]

Het enige dat ik niet zeker ben is of dit ook werkt op LibreSSL. Het zou kunnen dat sk_X509_pop_free daar niet bestaat. In dat geval zal er wss ergens een helper zijn in de PHP code of zal er een lelijker alternatief zijn. Maar je zal wel zien wat de PHP CI zegt en wat de maintainer zegt