Fix phpGH-20802: undefined behavior with invalid SNI_server_certs options.

close phpGH-20803

Author: devnexen

NEWS

@@ -38,6 +38,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-20674 (mb_decode_mimeheader does not handle separator).
     (Yuya Hamada)
 
+- OpenSSL:
+  . Fixed bug GH-20802 (undefined behavior with invalid SNI_server_certs
+    options). (David Carlier)
+
 - PCNTL:
   . Fixed bug with pcntl_getcpuaffinity() on solaris regarding invalid
     process ids handling. (David Carlier)

ext/openssl/tests/gh20802.phpt

@@ -0,0 +1,62 @@
+--TEST--
+GH-20802: undefined behavior with invalid SNI_server_certs option values
+--EXTENSIONS--
+openssl
+--SKIPIF--
+<?php
+if (!function_exists("proc_open")) die("skip no proc_open");
+?>
+--FILE--
+<?php
+$certFile = __DIR__ . DIRECTORY_SEPARATOR . 'gh20802.pem.tmp';
+$serverCode = <<<'CODE'
+    class A {};
+    $localhost = new A();
+    ini_set('log_errors', 'On');
+    $flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
+    $ctx = stream_context_create([
+	    'ssl' => [
+		    'local_cert'        => '%s',
+		    'allow_self_signed' => true,
+		    'verify_peer_name'  => false,
+		    'verify_peer'  => false,
+		    'SNI_enabled' => true,
+		    'SNI_server_certs' => [
+			    'localhost' => &$localhost,
+		    ]
+	    ]
+    ]);
+    $server = stream_socket_server('tls://127.0.0.1:12443', $errno, $errstr, $flags, $ctx);
+    phpt_notify_server_start($server);
+    stream_socket_accept($server, 3);
+CODE;
+$serverCode = sprintf($serverCode, $certFile);
+
+$clientCode = <<<'CODE'
+    $flags = STREAM_CLIENT_CONNECT;
+
+$ctx = stream_context_create([
+	'ssl' => [
+	    'SNI_enabled' => true,
+	    'verify_peer_name'  => false,
+	    'verify_peer'  => false
+	]
+    ]);
+    @stream_socket_client("tls://127.0.0.1:12443", $errno, $errstr, 1, $flags, $ctx);
+CODE;
+
+include 'CertificateGenerator.inc';
+$certificateGenerator = new CertificateGenerator();
+$certificateGenerator->saveNewCertAsFileWithKey('gh20802-snioptions', $certFile);
+
+include 'ServerClientTestCase.inc';
+ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh20802.pem.tmp');
+?>
+--EXPECTF--
+%a
+PHP Warning:  stream_socket_accept(): Failed to enable crypto in %s(%d) : eval()'d code on line %d
+PHP Warning:  stream_socket_accept(): Accept failed: Cannot enable crypto in %s(%d) : eval()'d code on line %d

ext/openssl/xp_ssl.c

@@ -1448,7 +1448,7 @@ static zend_result php_openssl_enable_server_sni(
 	zend_ulong key_index;
 	int i = 0;
 	char resolved_path_buff[MAXPATHLEN];
-	SSL_CTX *ctx;
+	SSL_CTX *ctx = NULL;
 
 	/* If the stream ctx disables SNI we're finished here */
 	if (GET_VER_OPT("SNI_enabled") && !zend_is_true(val)) {
@@ -1490,6 +1490,8 @@ static zend_result php_openssl_enable_server_sni(
 			return FAILURE;
 		}
 
+		ZVAL_DEREF(current);
+
 		if (Z_TYPE_P(current) == IS_ARRAY) {
 			zval *local_pk, *local_cert;
 			zend_string *local_pk_str, *local_cert_str;
@@ -1544,17 +1546,17 @@ static zend_result php_openssl_enable_server_sni(
 			zend_string_release(local_pk_str);
 
 			ctx = php_openssl_create_sni_server_ctx(resolved_cert_path_buff, resolved_pk_path_buff);
-
-		} else if (php_openssl_check_path_str_ex(
-				Z_STR_P(current), resolved_path_buff, 0, false, false,
-				"SNI_server_certs in ssl stream context")) {
-			ctx = php_openssl_create_sni_server_ctx(resolved_path_buff, resolved_path_buff);
+		} else if (Z_TYPE_P(current) == IS_STRING) {
+			if (php_openssl_check_path_str_ex(Z_STR_P(current), resolved_path_buff, 0, false, false, "SNI_server_certs in ssl stream context")) {
+				ctx = php_openssl_create_sni_server_ctx(resolved_path_buff, resolved_path_buff);
+			} else {
+				php_error_docref(NULL, E_WARNING,
+						"Failed setting local cert chain file `%s'; file not found",
+						Z_STRVAL_P(current)
+				);
+			}
 		} else {
-			php_error_docref(NULL, E_WARNING,
-				"Failed setting local cert chain file `%s'; file not found",
-				Z_STRVAL_P(current)
-			);
-			return FAILURE;
+			php_error_docref(NULL, E_WARNING, "SNI_server_certs options values must be of type array|string");
 		}
 
 		if (ctx == NULL) {