diff --git a/ext/openssl/tests/tls_psk_oversized_identity.phpt b/ext/openssl/tests/tls_psk_oversized_identity.phpt new file mode 100644 index 000000000000..48b7680fd4f2 --- /dev/null +++ b/ext/openssl/tests/tls_psk_oversized_identity.phpt @@ -0,0 +1,55 @@ +--TEST-- +TLS PSK - oversized identity from an unserialized Psk is rejected +--EXTENSIONS-- +openssl +--SKIPIF-- += 1.1.1 required"); +?> +--FILE-- + [ + 'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_2_SERVER, + 'ciphers' => 'PSK', + 'psk_server_cb' => function ($stream, string $identity): ?Openssl\Psk { + return new Openssl\Psk(str_repeat("\x42", 16)); + }, + ]]); + $server = stream_socket_server('tls://127.0.0.1:0', $errno, $errstr, + STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $serverCtx); + phpt_notify_server_start($server); + @stream_socket_accept($server, 3); +CODE; + +$clientCode = <<<'CODE' + // unserialize bypasses the constructor length check, so the Psk carries a + // 512-byte identity; the callback must reject it, not overflow the buffer. + $identity = str_repeat('A', 512); + $payload = sprintf('O:11:"Openssl\Psk":2:{s:3:"psk";s:4:"pass";s:8:"identity";s:%d:"%s";}', + strlen($identity), $identity); + + $clientCtx = stream_context_create(['ssl' => [ + 'crypto_method' => STREAM_CRYPTO_METHOD_TLSv1_2_CLIENT, + 'ciphers' => 'PSK', + 'verify_peer' => false, + 'verify_peer_name' => false, + 'psk_client_cb' => function ($stream) use ($payload): ?Openssl\Psk { + return unserialize($payload); + }, + ]]); + try { + @stream_socket_client('tls://{{ ADDR }}', $errno, $errstr, + 30, STREAM_CLIENT_CONNECT, $clientCtx); + echo "no exception\n"; + } catch (ValueError $e) { + echo "caught: ", $e->getMessage(), "\n"; + } +CODE; + +include __DIR__ . '/ServerClientTestCase.inc'; +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +caught: PSK identity must be shorter than %d bytes diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 5cead037715a..b8f330f91f8a 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -1772,8 +1772,6 @@ static unsigned int php_openssl_psk_client_cb(SSL *ssl, const char *hint, unsigned char *psk_out, unsigned int max_psk_len) { (void)hint; /* identity hint deliberately not exposed as it is useless */ - (void)max_identity_len; - (void)max_psk_len; php_stream *stream = (php_stream *)SSL_get_ex_data(ssl, php_openssl_get_ssl_stream_data_index()); @@ -1812,6 +1810,18 @@ static unsigned int php_openssl_psk_client_cb(SSL *ssl, const char *hint, return 0; } + /* Returned lengths are untrusted, so bound them to the buffers. */ + if (ZSTR_LEN(identity_str) >= max_identity_len) { + zend_value_error("PSK identity must be shorter than %u bytes", max_identity_len); + zval_ptr_dtor(&result); + return 0; + } + if (ZSTR_LEN(psk_str) > max_psk_len) { + zend_value_error("PSK key must not exceed %u bytes", max_psk_len); + zval_ptr_dtor(&result); + return 0; + } + memcpy(identity_out, ZSTR_VAL(identity_str), ZSTR_LEN(identity_str)); identity_out[ZSTR_LEN(identity_str)] = '\0'; memcpy(psk_out, ZSTR_VAL(psk_str), ZSTR_LEN(psk_str)); @@ -1824,8 +1834,6 @@ static unsigned int php_openssl_psk_client_cb(SSL *ssl, const char *hint, static unsigned int php_openssl_psk_server_cb(SSL *ssl, const char *identity, unsigned char *psk_out, unsigned int max_psk_len) { - (void)max_psk_len; - php_stream *stream = (php_stream *)SSL_get_ex_data(ssl, php_openssl_get_ssl_stream_data_index()); if (stream == NULL) { @@ -1865,6 +1873,13 @@ static unsigned int php_openssl_psk_server_cb(SSL *ssl, const char *identity, return 0; } + /* Returned length is untrusted, so bound it to the buffer. */ + if (ZSTR_LEN(psk_str) > max_psk_len) { + zend_value_error("PSK key must not exceed %u bytes", max_psk_len); + zval_ptr_dtor(&result); + return 0; + } + memcpy(psk_out, ZSTR_VAL(psk_str), ZSTR_LEN(psk_str)); unsigned int psk_len = (unsigned int)ZSTR_LEN(psk_str); @@ -1930,6 +1945,14 @@ static int php_openssl_psk_use_session_cb(SSL *ssl, const EVP_MD *md, return 0; } + /* Returned length is untrusted, so bound it to the buffer. */ + if (ZSTR_LEN(identity_str) > PHP_OPENSSL_PSK_MAX_IDENTITY_LEN) { + zend_value_error("PSK identity must not exceed %d bytes", PHP_OPENSSL_PSK_MAX_IDENTITY_LEN); + SSL_SESSION_free(session); + zval_ptr_dtor(&result); + return 0; + } + /* Identity buffer must outlive this callback. */ if (sslsock->psk_identity_buf == NULL) { sslsock->psk_identity_buf = emalloc(PHP_OPENSSL_PSK_MAX_IDENTITY_LEN);