Skip to content

Commit 70729c2

Browse files
committed
Fix GH-20802: undefined behavior with invalid SNI_server_certs options.
1 parent 7effcab commit 70729c2

File tree

2 files changed

+75
-9
lines changed

2 files changed

+75
-9
lines changed

ext/openssl/tests/gh20802.phpt

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
--TEST--
2+
GH-20802: undefined behavior with invalid SNI_server_certs option values
3+
--EXTENSIONS--
4+
openssl
5+
--SKIPIF--
6+
<?php
7+
if (!function_exists("proc_open")) die("skip no proc_open");
8+
?>
9+
--FILE--
10+
<?php
11+
$certFile = __DIR__ . DIRECTORY_SEPARATOR . 'gh20802.pem.tmp';
12+
$serverCode = <<<'CODE'
13+
class A {};
14+
$localhost = new A();
15+
ini_set('log_errors', 'On');
16+
$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
17+
$ctx = stream_context_create([
18+
'ssl' => [
19+
'local_cert' => '%s',
20+
'allow_self_signed' => true,
21+
'verify_peer_name' => false,
22+
'verify_peer' => false,
23+
'SNI_enabled' => true,
24+
'SNI_server_certs' => [
25+
'localhost' => &$localhost,
26+
]
27+
]
28+
]);
29+
$server = stream_socket_server('tls://127.0.0.1:12443', $errno, $errstr, $flags, $ctx);
30+
phpt_notify_server_start($server);
31+
stream_socket_accept($server, 3);
32+
CODE;
33+
$serverCode = sprintf($serverCode, $certFile);
34+
35+
$clientCode = <<<'CODE'
36+
$flags = STREAM_CLIENT_CONNECT;
37+
38+
$ctx = stream_context_create([
39+
'ssl' => [
40+
'SNI_enabled' => true,
41+
'verify_peer_name' => false,
42+
'verify_peer' => false
43+
]
44+
]);
45+
@stream_socket_client("tls://127.0.0.1:12443", $errno, $errstr, 1, $flags, $ctx);
46+
CODE;
47+
48+
include 'CertificateGenerator.inc';
49+
$certificateGenerator = new CertificateGenerator();
50+
$certificateGenerator->saveNewCertAsFileWithKey('gh20802-snioptions', $certFile);
51+
52+
include 'ServerClientTestCase.inc';
53+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
54+
?>
55+
--CLEAN--
56+
<?php
57+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh20802.pem.tmp');
58+
?>
59+
--EXPECTF--
60+
%a
61+
PHP Warning: stream_socket_accept(): Unable to set local cert chain file `%s'; Check that your cafile/capath settings include details of your certificate and its issuer in %s(%d) : eval()'d code on line %d
62+
PHP Warning: stream_socket_accept(): Failed to enable crypto in %s(%d) : eval()'d code on line %d
63+
PHP Warning: stream_socket_accept(): Accept failed: Cannot enable crypto in %s(%d) : eval()'d code on line %d

ext/openssl/xp_ssl.c

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,6 +1494,8 @@ static zend_result php_openssl_enable_server_sni(
14941494
return FAILURE;
14951495
}
14961496

1497+
ZVAL_DEREF(current);
1498+
14971499
if (Z_TYPE_P(current) == IS_ARRAY) {
14981500
zval *local_pk, *local_cert;
14991501
zend_string *local_pk_str, *local_cert_str;
@@ -1548,16 +1550,17 @@ static zend_result php_openssl_enable_server_sni(
15481550
zend_string_release(local_pk_str);
15491551

15501552
ctx = php_openssl_create_sni_server_ctx(resolved_cert_path_buff, resolved_pk_path_buff);
1551-
1552-
} else if (php_openssl_check_path_str_ex(
1553-
Z_STR_P(current), resolved_path_buff, 0, false, false,
1554-
"SNI_server_certs in ssl stream context")) {
1555-
ctx = php_openssl_create_sni_server_ctx(resolved_path_buff, resolved_path_buff);
1553+
} else if (Z_TYPE_P(current) == IS_STRING) {
1554+
if (php_openssl_check_path_str_ex(Z_STR_P(current), resolved_path_buff, 0, false, false, "SNI_server_certs in ssl stream context")) {
1555+
ctx = php_openssl_create_sni_server_ctx(resolved_path_buff, resolved_path_buff);
1556+
} else {
1557+
php_error_docref(NULL, E_WARNING,
1558+
"Failed setting local cert chain file `%s'; file not found",
1559+
Z_STRVAL_P(current)
1560+
);
1561+
}
15561562
} else {
1557-
php_error_docref(NULL, E_WARNING,
1558-
"Failed setting local cert chain file `%s'; file not found",
1559-
Z_STRVAL_P(current)
1560-
);
1563+
php_error_docref(NULL, E_WARNING, "SNI_server_certs options values must be of type array|string");
15611564
return FAILURE;
15621565
}
15631566

0 commit comments

Comments
 (0)