Skip to content

Commit 70c4419

Browse files
committed
Fix GH-20802: undefined behavior with invalid SNI_server_certs options.
1 parent 5ee2add commit 70c4419

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
@@ -1490,6 +1490,8 @@ static zend_result php_openssl_enable_server_sni(
14901490
return FAILURE;
14911491
}
14921492

1493+
ZVAL_DEREF(current);
1494+
14931495
if (Z_TYPE_P(current) == IS_ARRAY) {
14941496
zval *local_pk, *local_cert;
14951497
zend_string *local_pk_str, *local_cert_str;
@@ -1544,16 +1546,17 @@ static zend_result php_openssl_enable_server_sni(
15441546
zend_string_release(local_pk_str);
15451547

15461548
ctx = php_openssl_create_sni_server_ctx(resolved_cert_path_buff, resolved_pk_path_buff);
1547-
1548-
} else if (php_openssl_check_path_str_ex(
1549-
Z_STR_P(current), resolved_path_buff, 0, false, false,
1550-
"SNI_server_certs in ssl stream context")) {
1551-
ctx = php_openssl_create_sni_server_ctx(resolved_path_buff, resolved_path_buff);
1549+
} else if (Z_TYPE_P(current) == IS_STRING) {
1550+
if (php_openssl_check_path_str_ex(Z_STR_P(current), resolved_path_buff, 0, false, false, "SNI_server_certs in ssl stream context")) {
1551+
ctx = php_openssl_create_sni_server_ctx(resolved_path_buff, resolved_path_buff);
1552+
} else {
1553+
php_error_docref(NULL, E_WARNING,
1554+
"Failed setting local cert chain file `%s'; file not found",
1555+
Z_STRVAL_P(current)
1556+
);
1557+
}
15521558
} else {
1553-
php_error_docref(NULL, E_WARNING,
1554-
"Failed setting local cert chain file `%s'; file not found",
1555-
Z_STRVAL_P(current)
1556-
);
1559+
php_error_docref(NULL, E_WARNING, "SNI_server_certs options values must be of type array|string");
15571560
return FAILURE;
15581561
}
15591562

0 commit comments

Comments
 (0)