From 55717656097918baf21fe272a788db501ed33854 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 19 Jun 2020 09:27:19 +0200 Subject: [PATCH 1/4] Forbid use of +--EXPECTF-- +Parse error: Cannot use "" Date: Fri, 19 Jun 2020 09:43:56 +0200 Subject: [PATCH 2/4] Generate temporary config file when generating certificates The putenv trick doesn't work on ZTS Windows, so generate a new openssl config every time. --- ext/openssl/tests/CertificateGenerator.inc | 71 ++++++++++++++-------- ext/openssl/tests/san.cnf | 13 ---- 2 files changed, 47 insertions(+), 37 deletions(-) delete mode 100644 ext/openssl/tests/san.cnf diff --git a/ext/openssl/tests/CertificateGenerator.inc b/ext/openssl/tests/CertificateGenerator.inc index 4cd8540cefad7..b409376058efe 100644 --- a/ext/openssl/tests/CertificateGenerator.inc +++ b/ext/openssl/tests/CertificateGenerator.inc @@ -3,7 +3,6 @@ class CertificateGenerator { const CONFIG = __DIR__. DIRECTORY_SEPARATOR . 'openssl.cnf'; - const SAN_CONFIG = __DIR__ . DIRECTORY_SEPARATOR . 'san.cnf'; /** @var resource */ private $ca; @@ -96,32 +95,56 @@ class CertificateGenerator $dn['commonName'] = $commonNameForCert; } - $config = [ - 'digest_alg' => 'sha256', - 'req_extensions' => 'v3_req', - 'x509_extensions' => 'usr_cert', - ]; - if ($subjectAltName !== null) { - putenv("PHP_SUBJECTALTNAME=$subjectAltName"); - $config['config'] = self::SAN_CONFIG; - } - - $this->lastKey = self::generateKey($keyLength); - $this->lastCert = openssl_csr_sign( - openssl_csr_new($dn, $this->lastKey, $config), - $this->ca, - $this->caKey, - /* days */ 2, - $config, - ); + $subjectAltNameConfig = + $subjectAltName ? "subjectAltName = $subjectAltName" : ""; + $configCode = << $configFile, + 'req_extensions' => 'v3_req', + 'x509_extensions' => 'usr_cert', + ]; + + $this->lastKey = self::generateKey($keyLength); + $this->lastCert = openssl_csr_sign( + openssl_csr_new($dn, $this->lastKey, $config), + $this->ca, + $this->caKey, + /* days */ 2, + $config, + ); + if (!$this->lastCert) { + throw new Exception('Failed to create certificate'); + } - $certText = ''; - openssl_x509_export($this->lastCert, $certText); + $certText = ''; + openssl_x509_export($this->lastCert, $certText); - $keyText = ''; - openssl_pkey_export($this->lastKey, $keyText); + $keyText = ''; + openssl_pkey_export($this->lastKey, $keyText); - file_put_contents($file, $certText . PHP_EOL . $keyText); + file_put_contents($file, $certText . PHP_EOL . $keyText); + } finally { + unlink($configFile); + } } public function getCertDigest($algo) diff --git a/ext/openssl/tests/san.cnf b/ext/openssl/tests/san.cnf deleted file mode 100644 index fd347331a908c..0000000000000 --- a/ext/openssl/tests/san.cnf +++ /dev/null @@ -1,13 +0,0 @@ -[ req ] -distinguished_name = req_distinguished_name - -[ req_distinguished_name ] - -[ v3_req ] -basicConstraints = CA:FALSE -keyUsage = nonRepudiation, digitalSignature, keyEncipherment -subjectAltName = ${ENV::PHP_SUBJECTALTNAME} - -[ usr_cert ] -basicConstraints = CA:FALSE -subjectAltName = ${ENV::PHP_SUBJECTALTNAME} From 5d7ff25311f1c640a78ca9ac1b83a0366d4882d2 Mon Sep 17 00:00:00 2001 From: Paul Dragoonis Date: Thu, 18 Jun 2020 10:40:40 +0100 Subject: [PATCH 3/4] Removing HTML Functionality from run-tests.php As discussed on GH-5632, the HTML functionality does not appear to be in active use. For HTML rendering of test results, it is suggested to instead use the JUnit integration, in combination with your favorite JUnit viewer. Closes GH-5705. --- run-tests.php | 131 +++++--------------------------------------------- 1 file changed, 12 insertions(+), 119 deletions(-) diff --git a/run-tests.php b/run-tests.php index 656ba645a1b07..26bd7363166eb 100755 --- a/run-tests.php +++ b/run-tests.php @@ -87,15 +87,11 @@ function show_usage() --help -h This Help. - --html Generate HTML output. - --temp-source --temp-target [--temp-urlbase ] Write temporary files to by replacing from the - filenames to generate with . If --html is being used and - given then the generated links are relative and prefixed - with the given url. In general you want to make the path - to your source files and some patch in your web page - hierarchy with pointing to . + filenames to generate with . In general you want to make + the path to your source files and some patch in + your web page hierarchy with pointing to . --keep-[all|php|skip|clean] Do not delete 'all' files, 'php' test file, 'skip' or 'clean' @@ -137,7 +133,7 @@ function main() global $DETAILED, $PHP_FAILED_TESTS, $SHOW_ONLY_GROUPS, $argc, $argv, $cfg, $cfgfiles, $cfgtypes, $conf_passed, $end_time, $environment, $exts_skipped, $exts_tested, $exts_to_test, $failed_tests_file, - $html_file, $html_output, $ignored_by_ext, $ini_overwrites, $is_switch, + $ignored_by_ext, $ini_overwrites, $is_switch, $just_save_results, $log_format, $matches, $no_clean, $no_file_cache, $optionals, $output_file, $pass_option_n, $pass_options, $pattern_match, $php, $php_cgi, $phpdbg, $preload, $redir_tests, @@ -378,8 +374,6 @@ function main() $just_save_results = false; $valgrind = null; - $html_output = false; - $html_file = null; $temp_source = null; $temp_target = null; $temp_urlbase = null; @@ -606,10 +600,6 @@ function main() $repeat = true; } break; - case '--html': - $html_file = fopen($argv[++$i], 'wt'); - $html_output = is_resource($html_file); - break; case '--version': echo '$Id$' . "\n"; exit(1); @@ -691,20 +681,12 @@ function main() usort($test_files, "test_sort"); $start_time = time(); - if (!$html_output) { - echo "Running selected tests.\n"; - } else { - show_start($start_time); - } + echo "Running selected tests.\n"; $test_idx = 0; run_all_tests($test_files, $environment); $end_time = time(); - if ($html_output) { - show_end($end_time); - } - if ($failed_tests_file) { fclose($failed_tests_file); } @@ -719,15 +701,8 @@ function main() } compute_summary(); - if ($html_output) { - fwrite($html_file, "
\n" . get_summary(false, true)); - } echo "====================================================================="; - echo get_summary(false, false); - - if ($html_output) { - fclose($html_file); - } + echo get_summary(false); if ($output_file != '' && $just_save_results) { save_or_mail_results(); @@ -792,10 +767,6 @@ function main() show_end($end_time); show_summary(); - if ($html_output) { - fclose($html_file); - } - save_or_mail_results(); } @@ -962,7 +933,7 @@ function save_or_mail_results() $failed_tests_data = ''; $sep = "\n" . str_repeat('=', 80) . "\n"; $failed_tests_data .= $failed_test_summary . "\n"; - $failed_tests_data .= get_summary(true, false) . "\n"; + $failed_tests_data .= get_summary(true) . "\n"; if ($sum_results['FAILED']) { foreach ($PHP_FAILED_TESTS['FAILED'] as $test_info) { @@ -3015,7 +2986,7 @@ function compute_summary() } } -function get_summary($show_ext_summary, $show_html) +function get_summary($show_ext_summary) { global $exts_skipped, $exts_tested, $n_total, $sum_results, $percent_results, $end_time, $start_time, $failed_test_summary, $PHP_FAILED_TESTS, $valgrind; @@ -3034,10 +3005,6 @@ function get_summary($show_ext_summary, $show_html) $summary = ''; - if ($show_html) { - $summary .= "
\n";
-    }
-
     if ($show_ext_summary) {
         $summary .= '
 =====================================================================
@@ -3179,55 +3146,27 @@ function get_summary($show_ext_summary, $show_html)
         $summary .= $failed_test_summary;
     }
 
-    if ($show_html) {
-        $summary .= "
"; - } - return $summary; } function show_start($start_time) { - global $html_output, $html_file; - - if ($html_output) { - fwrite($html_file, "

Time Start: " . date('Y-m-d H:i:s', $start_time) . "

\n"); - fwrite($html_file, "\n"); - } - echo "TIME START " . date('Y-m-d H:i:s', $start_time) . "\n=====================================================================\n"; } function show_end($end_time) { - global $html_output, $html_file; - - if ($html_output) { - fwrite($html_file, "
\n"); - fwrite($html_file, "

Time End: " . date('Y-m-d H:i:s', $end_time) . "

\n"); - } - echo "=====================================================================\nTIME END " . date('Y-m-d H:i:s', $end_time) . "\n"; } function show_summary() { - global $html_output, $html_file; - - if ($html_output) { - fwrite($html_file, "
\n" . get_summary(true, true)); - } - - echo get_summary(true, false); + echo get_summary(true); } function show_redirect_start($tests, $tested, $tested_file) { - global $html_output, $html_file, $line_length, $SHOW_ONLY_GROUPS; - - if ($html_output) { - fwrite($html_file, "---> $tests ($tested [$tested_file]) begin\n"); - } + global $SHOW_ONLY_GROUPS; if (!$SHOW_ONLY_GROUPS || in_array('REDIRECT', $SHOW_ONLY_GROUPS)) { echo "REDIRECT $tests ($tested [$tested_file]) begin\n"; @@ -3238,11 +3177,7 @@ function show_redirect_start($tests, $tested, $tested_file) function show_redirect_ends($tests, $tested, $tested_file) { - global $html_output, $html_file, $line_length, $SHOW_ONLY_GROUPS; - - if ($html_output) { - fwrite($html_file, "---> $tests ($tested [$tested_file]) done\n"); - } + global $SHOW_ONLY_GROUPS; if (!$SHOW_ONLY_GROUPS || in_array('REDIRECT', $SHOW_ONLY_GROUPS)) { echo "REDIRECT $tests ($tested [$tested_file]) done\n"; @@ -3283,7 +3218,7 @@ function parse_conflicts(string $text): array function show_result($result, $tested, $tested_file, $extra = '', $temp_filenames = null) { - global $html_output, $html_file, $temp_target, $temp_urlbase, $line_length, $SHOW_ONLY_GROUPS; + global $temp_target, $temp_urlbase, $line_length, $SHOW_ONLY_GROUPS; if (!$SHOW_ONLY_GROUPS || in_array($result, $SHOW_ONLY_GROUPS)) { echo "$result $tested [$tested_file] $extra\n"; @@ -3291,48 +3226,6 @@ function show_result($result, $tested, $tested_file, $extra = '', $temp_filename clear_show_test(); } - if ($html_output) { - if (isset($temp_filenames['file']) && file_exists($temp_filenames['file'])) { - $url = str_replace($temp_target, $temp_urlbase, $temp_filenames['file']); - $tested = "$tested"; - } - - if (isset($temp_filenames['skip']) && file_exists($temp_filenames['skip'])) { - if (empty($extra)) { - $extra = "skipif"; - } - - $url = str_replace($temp_target, $temp_urlbase, $temp_filenames['skip']); - $extra = "$extra"; - } elseif (empty($extra)) { - $extra = " "; - } - - if (isset($temp_filenames['diff']) && file_exists($temp_filenames['diff'])) { - $url = str_replace($temp_target, $temp_urlbase, $temp_filenames['diff']); - $diff = "diff"; - } else { - $diff = " "; - } - - if (isset($temp_filenames['mem']) && file_exists($temp_filenames['mem'])) { - $url = str_replace($temp_target, $temp_urlbase, $temp_filenames['mem']); - $mem = "leaks"; - } else { - $mem = " "; - } - - fwrite( - $html_file, - "" . - "$result" . - "$tested" . - "$extra" . - "$diff" . - "$mem" . - "\n" - ); - } } function junit_init() From 32f377b0b94482a5b126408942646b9bd101c042 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 19 Jun 2020 10:46:02 +0200 Subject: [PATCH 4/4] Fixed bug #79710 Make sure we don't use zresource after the stream has been destroyed. --- NEWS | 4 ++++ ext/spl/spl_directory.c | 9 +++++++-- ext/spl/tests/bug79710.phpt | 40 +++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 ext/spl/tests/bug79710.phpt diff --git a/NEWS b/NEWS index c7478ce2de06a..33fbff3a54f27 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,10 @@ PHP NEWS . Fixed bug #79664 (PDOStatement::getColumnMeta fails on empty result set). (cmb) +- SPL: + . Fixed bug #79710 (Reproducible segfault in error_handler during GC + involved an SplFileObject). (Nikita) + - Standard: . Fixed bug #74267 (segfault with streams and invalid data). (cmb) diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index b9e6bf91e8b43..6e79bc407fef2 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -96,6 +96,7 @@ static void spl_filesystem_object_destroy_object(zend_object *object) /* {{{ */ php_stream_pclose(intern->u.file.stream); } intern->u.file.stream = NULL; + ZVAL_UNDEF(&intern->u.file.zresource); } break; default: @@ -2062,12 +2063,16 @@ static int spl_filesystem_file_call(spl_filesystem_object *intern, zend_function { zend_fcall_info fci; zend_fcall_info_cache fcic; - zval *zresource_ptr = &intern->u.file.zresource, retval; + zval *zresource_ptr = &intern->u.file.zresource, *params, retval; int result; int num_args = pass_num_args + (arg2 ? 2 : 1); - zval *params = (zval*)safe_emalloc(num_args, sizeof(zval), 0); + if (Z_ISUNDEF_P(zresource_ptr)) { + zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized"); + return FAILURE; + } + params = (zval*)safe_emalloc(num_args, sizeof(zval), 0); params[0] = *zresource_ptr; if (arg2) { diff --git a/ext/spl/tests/bug79710.phpt b/ext/spl/tests/bug79710.phpt new file mode 100644 index 0000000000000..a5c065fd67ba6 --- /dev/null +++ b/ext/spl/tests/bug79710.phpt @@ -0,0 +1,40 @@ +--TEST-- +Bug #79710: Reproducible segfault in error_handler during GC involved an SplFileObject +--FILE-- +sfo = $sfo; + } + public function __destruct() { + // If the SplFileObject is destructed first, + // underlying FD is no longer valid and will cause error upon calling flock + $this->sfo->flock(2); + } +} + +class Run +{ + static $sfo; + static $foo; + public static function main() { + // Creation ordering is important for repro + // $sfo needed to be destructed before $foo. + Run::$sfo = new SplTempFileObject(); + Run::$foo = new Target(Run::$sfo); + } +} + +Run::main(); + +?> +--EXPECTF-- +Fatal error: Uncaught RuntimeException: Object not initialized in %s:%d +Stack trace: +#0 %s(%d): SplFileObject->flock(2) +#1 [internal function]: Target->__destruct() +#2 {main} + thrown in %s on line %d