Skip to content

Commit 591fe92

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.2' into PHP-8.3
* PHP-8.2: Fix memory leak in php_openssl_pkey_from_zval() Fix various memory leaks related to openssl exports Prevent unexpected array entry conversion when reading key
2 parents 73c4fa0 + 994e866 commit 591fe92

7 files changed

+121
-13
lines changed

‎NEWS

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ PHP NEWS
88
- FPM:
99
. Fixed GH-16432 (PHP-FPM 8.2 SIGSEGV in fpm_get_status). (Jakub Zelenka)
1010

11+
- OpenSSL:
12+
. Prevent unexpected array entry conversion when reading key. (nielsdos)
13+
. Fix various memory leaks related to openssl exports. (nielsdos)
14+
. Fix memory leak in php_openssl_pkey_from_zval(). (nielsdos)
15+
1116
- PDO:
1217
. Fixed memory leak of `setFetchMode()`. (SakiTakamachi)
1318

‎ext/openssl/openssl.c

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,7 +1538,7 @@ PHP_FUNCTION(openssl_x509_export_to_file)
15381538
}
15391539

15401540
if (!php_openssl_check_path(filename, filename_len, file_path, 2)) {
1541-
return;
1541+
goto exit_cleanup_cert;
15421542
}
15431543

15441544
bio_out = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY));
@@ -1556,13 +1556,14 @@ PHP_FUNCTION(openssl_x509_export_to_file)
15561556
php_error_docref(NULL, E_WARNING, "Error opening file %s", file_path);
15571557
}
15581558

1559-
if (cert_str) {
1560-
X509_free(cert);
1561-
}
1562-
15631559
if (!BIO_free(bio_out)) {
15641560
php_openssl_store_errors();
15651561
}
1562+
1563+
exit_cleanup_cert:
1564+
if (cert_str) {
1565+
X509_free(cert);
1566+
}
15661567
}
15671568
/* }}} */
15681569

@@ -3110,7 +3111,7 @@ PHP_FUNCTION(openssl_csr_export_to_file)
31103111
}
31113112

31123113
if (!php_openssl_check_path(filename, filename_len, file_path, 2)) {
3113-
return;
3114+
goto exit_cleanup;
31143115
}
31153116

31163117
bio_out = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY));
@@ -3130,6 +3131,7 @@ PHP_FUNCTION(openssl_csr_export_to_file)
31303131
php_error_docref(NULL, E_WARNING, "Error opening file %s", file_path);
31313132
}
31323133

3134+
exit_cleanup:
31333135
if (csr_str) {
31343136
X509_REQ_free(csr);
31353137
}
@@ -3571,6 +3573,7 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
35713573
} else {
35723574
ZVAL_COPY(&tmp, zphrase);
35733575
if (!try_convert_to_string(&tmp)) {
3576+
zval_ptr_dtor(&tmp);
35743577
return NULL;
35753578
}
35763579

@@ -3617,12 +3620,14 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36173620
if (!(Z_TYPE_P(val) == IS_STRING || Z_TYPE_P(val) == IS_OBJECT)) {
36183621
TMP_CLEAN;
36193622
}
3620-
if (!try_convert_to_string(val)) {
3623+
zend_string *val_str = zval_try_get_string(val);
3624+
if (!val_str) {
36213625
TMP_CLEAN;
36223626
}
36233627

3624-
if (Z_STRLEN_P(val) > 7 && memcmp(Z_STRVAL_P(val), "file://", sizeof("file://") - 1) == 0) {
3625-
if (!php_openssl_check_path_str(Z_STR_P(val), file_path, arg_num)) {
3628+
if (ZSTR_LEN(val_str) > 7 && memcmp(ZSTR_VAL(val_str), "file://", sizeof("file://") - 1) == 0) {
3629+
if (!php_openssl_check_path_str(val_str, file_path, arg_num)) {
3630+
zend_string_release_ex(val_str, false);
36263631
TMP_CLEAN;
36273632
}
36283633
is_file = true;
@@ -3641,10 +3646,11 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36413646
if (is_file) {
36423647
in = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY));
36433648
} else {
3644-
in = BIO_new_mem_buf(Z_STRVAL_P(val), (int)Z_STRLEN_P(val));
3649+
in = BIO_new_mem_buf(ZSTR_VAL(val_str), (int)ZSTR_LEN(val_str));
36453650
}
36463651
if (in == NULL) {
36473652
php_openssl_store_errors();
3653+
zend_string_release_ex(val_str, false);
36483654
TMP_CLEAN;
36493655
}
36503656
key = PEM_read_bio_PUBKEY(in, NULL,NULL, NULL);
@@ -3657,10 +3663,11 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36573663
if (is_file) {
36583664
in = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY));
36593665
} else {
3660-
in = BIO_new_mem_buf(Z_STRVAL_P(val), (int)Z_STRLEN_P(val));
3666+
in = BIO_new_mem_buf(ZSTR_VAL(val_str), (int)ZSTR_LEN(val_str));
36613667
}
36623668

36633669
if (in == NULL) {
3670+
zend_string_release_ex(val_str, false);
36643671
TMP_CLEAN;
36653672
}
36663673
if (passphrase == NULL) {
@@ -3673,6 +3680,8 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36733680
}
36743681
BIO_free(in);
36753682
}
3683+
3684+
zend_string_release_ex(val_str, false);
36763685
}
36773686

36783687
if (key == NULL) {
@@ -4749,7 +4758,7 @@ PHP_FUNCTION(openssl_pkey_export_to_file)
47494758
}
47504759

47514760
if (!php_openssl_check_path(filename, filename_len, file_path, 2)) {
4752-
RETURN_FALSE;
4761+
goto clean_exit_key;
47534762
}
47544763

47554764
PHP_SSL_REQ_INIT(&req);
@@ -4785,8 +4794,9 @@ PHP_FUNCTION(openssl_pkey_export_to_file)
47854794

47864795
clean_exit:
47874796
PHP_SSL_REQ_DISPOSE(&req);
4788-
EVP_PKEY_free(key);
47894797
BIO_free(bio_out);
4798+
clean_exit_key:
4799+
EVP_PKEY_free(key);
47904800
}
47914801
/* }}} */
47924802

‎ext/openssl/tests/openssl_csr_export_to_file_leak.phpt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
openssl_csr_export_to_file memory leak
3+
--EXTENSIONS--
4+
openssl
5+
--FILE--
6+
<?php
7+
8+
$path = "file://" . __DIR__ . "/cert.csr";
9+
var_dump(openssl_csr_export_to_file($path, str_repeat("a", 10000)));
10+
11+
?>
12+
--EXPECTF--
13+
Warning: openssl_csr_export_to_file(output_filename): must be a valid file path %s
14+
bool(false)

‎ext/openssl/tests/openssl_pkey_export_to_file_leak.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
openssl_pkey_export_to_file memory leak
3+
--EXTENSIONS--
4+
openssl
5+
--FILE--
6+
<?php
7+
8+
$path = "file://" . __DIR__ . "/private_rsa_1024.key";
9+
$key = [$path, ""];
10+
var_dump(openssl_pkey_export_to_file($key, str_repeat("a", 10000), passphrase: ""));
11+
12+
?>
13+
--EXPECTF--
14+
Warning: openssl_pkey_export_to_file(output_filename): must be a valid file path %s
15+
bool(false)

‎ext/openssl/tests/openssl_pkey_export_to_file_object_to_string.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
openssl_pkey_export_to_file object to string conversion
3+
--EXTENSIONS--
4+
openssl
5+
--FILE--
6+
<?php
7+
8+
class Test {
9+
public function __toString(): string {
10+
return "file://" . __DIR__ . "/private_rsa_1024.key";
11+
}
12+
}
13+
14+
$path = new Test;
15+
$key = [$path, ""];
16+
@openssl_pkey_export_to_file($key, str_repeat("a", 10000), passphrase: "");
17+
var_dump($key);
18+
19+
?>
20+
--EXPECT--
21+
array(2) {
22+
[0]=>
23+
object(Test)#1 (0) {
24+
}
25+
[1]=>
26+
string(0) ""
27+
}

‎ext/openssl/tests/openssl_x509_export_to_file_leak.phpt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
--TEST--
2+
openssl_x509_export_to_file memory leak
3+
--EXTENSIONS--
4+
openssl
5+
--FILE--
6+
<?php
7+
8+
$path = "file://" . __DIR__ . "/sni_server_ca.pem";
9+
var_dump(openssl_x509_export_to_file($path, str_repeat("a", 10000)));
10+
11+
?>
12+
--EXPECTF--
13+
Warning: openssl_x509_export_to_file(output_filename): must be a valid file path %s
14+
bool(false)

‎ext/openssl/tests/php_openssl_pkey_from_zval_leak.phpt

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
php_openssl_pkey_from_zval memory leak
3+
--EXTENSIONS--
4+
openssl
5+
--FILE--
6+
<?php
7+
8+
class StrFail {
9+
public function __toString(): string {
10+
throw new Error('create a leak');
11+
}
12+
}
13+
14+
$key = ["", new StrFail];
15+
try {
16+
openssl_pkey_export_to_file($key, "doesnotmatter");
17+
} catch (Error $e) {
18+
echo $e->getMessage(), "\n";
19+
}
20+
21+
?>
22+
--EXPECT--
23+
create a leak

0 commit comments

Comments
 (0)