Skip to content

Commit fd68e9b

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.3' into PHP-8.4
* PHP-8.3: 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 a85a5ef + 591fe92 commit fd68e9b

6 files changed

+116
-13
lines changed

‎ext/openssl/openssl.c

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,7 +1562,7 @@ PHP_FUNCTION(openssl_x509_export_to_file)
15621562
}
15631563

15641564
if (!php_openssl_check_path(filename, filename_len, file_path, 2)) {
1565-
return;
1565+
goto exit_cleanup_cert;
15661566
}
15671567

15681568
bio_out = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY));
@@ -1580,13 +1580,14 @@ PHP_FUNCTION(openssl_x509_export_to_file)
15801580
php_error_docref(NULL, E_WARNING, "Error opening file %s", file_path);
15811581
}
15821582

1583-
if (cert_str) {
1584-
X509_free(cert);
1585-
}
1586-
15871583
if (!BIO_free(bio_out)) {
15881584
php_openssl_store_errors();
15891585
}
1586+
1587+
exit_cleanup_cert:
1588+
if (cert_str) {
1589+
X509_free(cert);
1590+
}
15901591
}
15911592
/* }}} */
15921593

@@ -3143,7 +3144,7 @@ PHP_FUNCTION(openssl_csr_export_to_file)
31433144
}
31443145

31453146
if (!php_openssl_check_path(filename, filename_len, file_path, 2)) {
3146-
return;
3147+
goto exit_cleanup;
31473148
}
31483149

31493150
bio_out = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_W(PKCS7_BINARY));
@@ -3163,6 +3164,7 @@ PHP_FUNCTION(openssl_csr_export_to_file)
31633164
php_error_docref(NULL, E_WARNING, "Error opening file %s", file_path);
31643165
}
31653166

3167+
exit_cleanup:
31663168
if (csr_str) {
31673169
X509_REQ_free(csr);
31683170
}
@@ -3629,6 +3631,7 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36293631
} else {
36303632
ZVAL_COPY(&tmp, zphrase);
36313633
if (!try_convert_to_string(&tmp)) {
3634+
zval_ptr_dtor(&tmp);
36323635
return NULL;
36333636
}
36343637

@@ -3675,12 +3678,14 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36753678
if (!(Z_TYPE_P(val) == IS_STRING || Z_TYPE_P(val) == IS_OBJECT)) {
36763679
TMP_CLEAN;
36773680
}
3678-
if (!try_convert_to_string(val)) {
3681+
zend_string *val_str = zval_try_get_string(val);
3682+
if (!val_str) {
36793683
TMP_CLEAN;
36803684
}
36813685

3682-
if (Z_STRLEN_P(val) > 7 && memcmp(Z_STRVAL_P(val), "file://", sizeof("file://") - 1) == 0) {
3683-
if (!php_openssl_check_path_str(Z_STR_P(val), file_path, arg_num)) {
3686+
if (ZSTR_LEN(val_str) > 7 && memcmp(ZSTR_VAL(val_str), "file://", sizeof("file://") - 1) == 0) {
3687+
if (!php_openssl_check_path_str(val_str, file_path, arg_num)) {
3688+
zend_string_release_ex(val_str, false);
36843689
TMP_CLEAN;
36853690
}
36863691
is_file = true;
@@ -3699,10 +3704,11 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
36993704
if (is_file) {
37003705
in = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY));
37013706
} else {
3702-
in = BIO_new_mem_buf(Z_STRVAL_P(val), (int)Z_STRLEN_P(val));
3707+
in = BIO_new_mem_buf(ZSTR_VAL(val_str), (int)ZSTR_LEN(val_str));
37033708
}
37043709
if (in == NULL) {
37053710
php_openssl_store_errors();
3711+
zend_string_release_ex(val_str, false);
37063712
TMP_CLEAN;
37073713
}
37083714
key = PEM_read_bio_PUBKEY(in, NULL,NULL, NULL);
@@ -3715,10 +3721,11 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
37153721
if (is_file) {
37163722
in = BIO_new_file(file_path, PHP_OPENSSL_BIO_MODE_R(PKCS7_BINARY));
37173723
} else {
3718-
in = BIO_new_mem_buf(Z_STRVAL_P(val), (int)Z_STRLEN_P(val));
3724+
in = BIO_new_mem_buf(ZSTR_VAL(val_str), (int)ZSTR_LEN(val_str));
37193725
}
37203726

37213727
if (in == NULL) {
3728+
zend_string_release_ex(val_str, false);
37223729
TMP_CLEAN;
37233730
}
37243731
if (passphrase == NULL) {
@@ -3731,6 +3738,8 @@ static EVP_PKEY *php_openssl_pkey_from_zval(
37313738
}
37323739
BIO_free(in);
37333740
}
3741+
3742+
zend_string_release_ex(val_str, false);
37343743
}
37353744

37363745
if (key == NULL) {
@@ -4909,7 +4918,7 @@ PHP_FUNCTION(openssl_pkey_export_to_file)
49094918
}
49104919

49114920
if (!php_openssl_check_path(filename, filename_len, file_path, 2)) {
4912-
RETURN_FALSE;
4921+
goto clean_exit_key;
49134922
}
49144923

49154924
PHP_SSL_REQ_INIT(&req);
@@ -4945,8 +4954,9 @@ PHP_FUNCTION(openssl_pkey_export_to_file)
49454954

49464955
clean_exit:
49474956
PHP_SSL_REQ_DISPOSE(&req);
4948-
EVP_PKEY_free(key);
49494957
BIO_free(bio_out);
4958+
clean_exit_key:
4959+
EVP_PKEY_free(key);
49504960
}
49514961
/* }}} */
49524962

‎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)