ext/session: Close save handler when session encoding fails

NEWS

@@ -34,6 +34,8 @@ PHP                                                                        NEWS
     CLOCK_MONOTONIC_RAW. (nicolas-grekas)
   . Fixed array_map optimization with non-literal function or non-literal args.
     (Arnaud)
+  . Fixed use-after-free when a dl()-loaded extension declares a frameless
+    function and a later request calls it. (Ilia Alshanetsky)
 
 - FFI:
   . Fixed crashes with FFI callbacks created from __call() trampolines

Zend/zend_API.c

@@ -3089,7 +3089,11 @@ ZEND_API zend_result zend_register_functions(zend_class_entry *scope, const zend
 		internal_function->prototype = NULL;
 		internal_function->prop_info = NULL;
 		internal_function->attributes = NULL;
-		internal_function->frameless_function_infos = ptr->frameless_function_infos;
+		if (type == MODULE_TEMPORARY) {
+			internal_function->frameless_function_infos = NULL;
+		} else {
+			internal_function->frameless_function_infos = ptr->frameless_function_infos;
+		}
 		if (EG(active)) { // at run-time: this ought to only happen if registered with dl() or somehow temporarily at runtime
 			ZEND_MAP_PTR_INIT(internal_function->run_time_cache, zend_arena_calloc(&CG(arena), 1, zend_internal_run_time_cache_reserved_size()));
 		} else {

ext/curl/config.m4

@@ -24,6 +24,7 @@ if test "$PHP_CURL" != "no"; then
 
       AC_RUN_IFELSE([AC_LANG_PROGRAM([
 #include <stdio.h>
+#include <string.h>
 #include <strings.h>
 #include <curl/curl.h>
 ], [

ext/dl_test/dl_test.c

@@ -50,6 +50,28 @@ PHP_FUNCTION(dl_test_test2)
 }
 /* }}}*/
 
+PHP_FUNCTION(dl_test_frameless)
+{
+	zend_long value;
+
+	ZEND_PARSE_PARAMETERS_START(1, 1)
+		Z_PARAM_LONG(value)
+	ZEND_PARSE_PARAMETERS_END();
+
+	RETURN_LONG(value);
+}
+
+ZEND_FRAMELESS_FUNCTION(dl_test_frameless, 1)
+{
+	zend_long value;
+
+	Z_FLF_PARAM_LONG(1, value);
+
+	RETVAL_LONG(value);
+
+flf_clean:;
+}
+
 /* {{{ PHP_DL_TEST_USE_REGISTER_FUNCTIONS_DIRECTLY */
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_dl_test_use_register_functions_directly, 0, 0, IS_STRING, 0)
 ZEND_END_ARG_INFO()

ext/dl_test/dl_test.stub.php

@@ -11,6 +11,11 @@ function dl_test_test1(): void {}
 
 function dl_test_test2(string $str = ""): string {}
 
+/**
+ * @frameless-function {"arity": 1}
+ */
+function dl_test_frameless(int $value): int {}
+
 class DlTest {
     public function test(string $str = ""): string {}
 }

ext/dl_test/tests/frameless_temporary.inc

@@ -0,0 +1,8 @@
+<?php
+var_dump(dl_test_frameless(7));
+try {
+    dl_test_frameless("nope");
+    echo "no throw\n";
+} catch (TypeError $e) {
+    echo $e->getMessage(), "\n";
+}

ext/dl_test/tests/frameless_temporary.phpt

@@ -0,0 +1,53 @@
+--TEST--
+dl() of a frameless function does not keep the freed function record
+--SKIPIF--
+<?php
+if (PHP_OS_FAMILY === 'Windows') {
+    die('skip setarch -R is required so dl() remaps the extension at the stale handler address');
+}
+if (!getenv('TEST_PHP_CGI_EXECUTABLE')) {
+    die('skip php-cgi not available');
+}
+$setarch = trim((string) shell_exec('command -v setarch'));
+if ($setarch === '') {
+    die('skip setarch -R is required so dl() remaps the extension at the stale handler address');
+}
+$arch = php_uname('m');
+exec($setarch . ' ' . escapeshellarg($arch) . ' -R true', $setarch_out, $setarch_code);
+if ($setarch_code !== 0) {
+    die('skip setarch -R cannot run on ' . $arch);
+}
+$so = ini_get('extension_dir') . DIRECTORY_SEPARATOR . 'dl_test.so';
+if (!file_exists($so)) {
+    die('skip dl_test extension is not built (tried ' . $so . ')');
+}
+?>
+--FILE--
+<?php
+$cmd = 'env -u SCRIPT_FILENAME -u PATH_TRANSLATED -u REDIRECT_STATUS -u REQUEST_METHOD -u QUERY_STRING'
+    . ' USE_ZEND_ALLOC=0 ASAN_OPTIONS=' . escapeshellarg('detect_leaks=0:halt_on_error=1:abort_on_error=1')
+    . ' setarch ' . escapeshellarg(php_uname('m')) . ' -R '
+    . escapeshellarg(getenv('TEST_PHP_CGI_EXECUTABLE'))
+    . ' -n -q -T 2 -d enable_dl=1 -d extension_dir=' . escapeshellarg(ini_get('extension_dir'))
+    . ' ' . escapeshellarg(__DIR__ . '/frameless_temporary_cgi.inc');
+$proc = proc_open($cmd, [
+    0 => ['pipe', 'r'],
+    1 => ['pipe', 'w'],
+    2 => ['pipe', 'w'],
+], $pipes);
+fclose($pipes[0]);
+$out = stream_get_contents($pipes[1]);
+stream_get_contents($pipes[2]);
+fclose($pipes[1]);
+fclose($pipes[2]);
+echo $out;
+$code = proc_close($proc);
+if ($code !== 0) {
+    echo "exit:$code\n";
+}
+?>
+--EXPECT--
+int(7)
+dl_test_frameless(): Argument #1 ($value) must be of type int, string given
+int(7)
+dl_test_frameless(): Argument #1 ($value) must be of type int, string given

ext/dl_test/tests/frameless_temporary_cgi.inc

@@ -0,0 +1,7 @@
+<?php
+$ext = PHP_OS_FAMILY === 'Windows' ? 'php_dl_test.dll' : 'dl_test.so';
+if (!dl($ext)) {
+    echo "dl failed\n";
+    return;
+}
+include __DIR__ . '/frameless_temporary.inc';

ext/dl_test/tests/frameless_temporary_opcache.phpt

@@ -0,0 +1,19 @@
+--TEST--
+dl() does not compile calls to a temporary module's function as frameless
+--SKIPIF--
+<?php include __DIR__ . "/skip.inc"; ?>
+--EXTENSIONS--
+opcache
+--INI--
+enable_dl=1
+opcache.enable_cli=1
+opcache.opt_debug_level=0x10000
+--FILE--
+<?php
+dl(PHP_OS_FAMILY === 'Windows' ? 'php_dl_test.dll' : 'dl_test.so');
+include __DIR__ . '/frameless_temporary.inc';
+?>
+--EXPECTF--
+%A0001 INIT_FCALL 1 %d string("dl_test_frameless")
+%Aint(7)
+dl_test_frameless(): Argument #1 ($value) must be of type int, string given

ext/session/session.c

@@ -526,6 +526,7 @@ static void php_session_save_current_state(bool write)
 				/* Not being able to encode the session data means there is some kind of issue that prevents a write
 				 * (e.g. a key containing the '|' character with the default serialization) */
 				if (UNEXPECTED(val == NULL)) {
+					PS(mod)->s_close(&PS(mod_data));
 					return;
 				}
 

ext/session/tests/session_write_close_encode_failure.phpt

@@ -0,0 +1,76 @@
+--TEST--
+session_write_close() closes the save handler when encoding the session data fails
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--INI--
+session.use_cookies=0
+session.cache_limiter=
+session.serialize_handler=php
+session.save_handler=files
+--FILE--
+<?php
+ob_start();
+$dir = __DIR__ . '/session_write_close_encode_failure';
+@mkdir($dir);
+session_save_path($dir);
+session_start();
+$file = $dir . '/sess_' . session_id();
+$_SESSION['bad|key'] = 'value';
+var_dump(session_write_close());
+
+$fp = fopen($file, 'r');
+var_dump(flock($fp, LOCK_EX | LOCK_NB));
+fclose($fp);
+
+session_start();
+$file = $dir . '/sess_' . session_id();
+$_SESSION['closure'] = function () {};
+$_SESSION['bad|key'] = 'value';
+try {
+    session_write_close();
+} catch (Exception $e) {
+    echo $e::class, ": ", $e->getMessage(), PHP_EOL;
+}
+
+$fp = fopen($file, 'r');
+var_dump(flock($fp, LOCK_EX | LOCK_NB));
+fclose($fp);
+
+class Handler extends SessionHandler
+{
+    public function close(): bool
+    {
+        echo "close\n";
+        return parent::close();
+    }
+}
+
+session_set_save_handler(new Handler());
+session_start();
+$_SESSION['bad|key'] = 'value';
+var_dump(session_write_close());
+echo "done\n";
+?>
+--CLEAN--
+<?php
+$dir = __DIR__ . '/session_write_close_encode_failure';
+foreach (glob($dir . '/sess_*') as $file) {
+    unlink($file);
+}
+@rmdir($dir);
+?>
+--EXPECTF--
+Warning: session_write_close(): Failed to write session data. Data contains invalid key "bad|key" in %s on line %d
+bool(true)
+bool(true)
+
+Warning: session_write_close(): Failed to write session data. Data contains invalid key "bad|key" in %s on line %d
+Exception: Serialization of 'Closure' is not allowed
+bool(true)
+
+Warning: session_write_close(): Failed to write session data. Data contains invalid key "bad|key" in %s on line %d
+close
+bool(true)
+done

ext/soap/tests/bugs/bug62900.phpt

@@ -46,18 +46,15 @@ $combinations = [
 
 chdir(__DIR__);
 
-$args = ["-d", "display_startup_errors=0", "-d", "extension_dir=" . ini_get("extension_dir"), "-d", "extension=" . (substr(PHP_OS, 0, 3) == "WIN" ? "php_" : "") . "soap." . PHP_SHLIB_SUFFIX];
-if (php_ini_loaded_file()) {
-    // Necessary such that it works from a development directory in which case extension_dir might not be the real extension dir
-    $args[] = "-c";
-    $args[] = php_ini_loaded_file();
-}
+$php = getenv('TEST_PHP_EXECUTABLE');
+$args = getenv('TEST_PHP_EXTRA_ARGS');
+$cmd = "$php $args " . __DIR__ . '/bug62900_run';
 
 foreach ($combinations as list($wsdl, $xsd)) {
     file_put_contents(__DIR__."/bug62900.wsdl", $wsdl);
     file_put_contents(__DIR__."/bug62900.xsd", $xsd);
 
-    $proc = proc_open([PHP_BINARY, ...$args, __DIR__.'/bug62900_run'], [1 => ["pipe", "w"], 2 => ["pipe", "w"]], $pipes);
+    $proc = proc_open($cmd, [1 => ["pipe", "w"], 2 => ["pipe", "w"]], $pipes);
     echo stream_get_contents($pipes[1]);
     fclose($pipes[1]);
     proc_close($proc);

sapi/cli/tests/php_cli_server_ipv6_error_message.phpt

@@ -14,8 +14,12 @@ $descriptorspec = array(
     2 => array("pipe", "w")
 );
 
+$php = getenv('TEST_PHP_EXECUTABLE_ESCAPED');
+$args = getenv('TEST_PHP_EXTRA_ARGS');
+$cmd = "$php $args" . ' -S "[2001:db8::]:8080"';
+
 $process = proc_open(
-    PHP_BINARY . ' -S "[2001:db8::]:8080"',
+    $cmd,
     $descriptorspec,
     $pipes
 );