From 0bd513e1156812b502712ba7c9694ad67aea2c1f Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 21 Oct 2026 14:40:02 +0200 Subject: [PATCH] Reset serialization state before request input processing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Serialization and unserialization contexts are stored in module globals, but their allocations belong to the current request heap. If a bailout skips the normal context cleanup, those globals can retain pointers to memory that is freed at request shutdown. The next request can process input before module RINIT runs. In particular, multipart upload-progress handling can invoke session serialization during input parsing, before the existing RINIT reset. It may therefore observe the previous request’s stale context and lock state. Resetting only in RINIT is too late to protect this path. Clear the serialization lock and both context structures at the beginning of request startup, before input processing. This discards the bookkeeping without following potentially invalid pointers or running destructors. Remove the redundant reset from basic-module RINIT. Add regression coverage that abandons serialize or unserialize state via a bailout, then sends a multipart upload-progress request to the same worker. Verify the follow-up request sees cleared state for locked and unlocked cases. Validation: the regression passes with the startup reset alone and fails when both resets are removed. --- Zend/tests/arginfo_zpp_mismatch.inc | 1 + ext/zend_test/test.c | 33 ++++ ext/zend_test/test.stub.php | 4 + ext/zend_test/test_arginfo.h | 13 +- ext/zend_test/test_decl.h | 8 +- ext/zend_test/test_legacy_arginfo.h | 13 +- .../serialization_context_request_reset.phpt | 172 ++++++++++++++++++ 7 files changed, 238 insertions(+), 6 deletions(-) create mode 100644 sapi/cli/tests/serialization_context_request_reset.phpt diff --git a/Zend/tests/arginfo_zpp_mismatch.inc b/Zend/tests/arginfo_zpp_mismatch.inc index 9c03e719c2a9..695486d23132 100644 --- a/Zend/tests/arginfo_zpp_mismatch.inc +++ b/Zend/tests/arginfo_zpp_mismatch.inc @@ -10,6 +10,7 @@ function skipFunction($function): bool { || $function === 'exit' || $function === 'die' || $function === 'zend_trigger_bailout' + || $function === 'zend_test_serialization_bailout' /* intentionally violate invariants */ || $function === 'zend_create_unterminated_string' || $function === 'zend_test_array_return' diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 846fd36a4038..17312fca566f 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -21,6 +21,7 @@ #include "php.h" #include "php_ini.h" #include "ext/standard/info.h" +#include "ext/standard/php_var.h" #include "php_test.h" #include "observer.h" #include "fiber.h" @@ -212,6 +213,38 @@ static ZEND_FUNCTION(zend_leak_bytes) emalloc(leakbytes); } +static ZEND_FUNCTION(zend_test_serialization_bailout) +{ + bool unserialize, locked; + + ZEND_PARSE_PARAMETERS_START(2, 2) + Z_PARAM_BOOL(unserialize) + Z_PARAM_BOOL(locked) + ZEND_PARSE_PARAMETERS_END(); + + if (unserialize) { + (void) php_var_unserialize_init(); + } else { + (void) php_var_serialize_init(); + } + if (locked) { + BG(serialize_lock)++; + } + zend_bailout(); +} + +static ZEND_FUNCTION(zend_test_serialization_state) +{ + ZEND_PARSE_PARAMETERS_NONE(); + + array_init(return_value); + add_assoc_long(return_value, "lock", BG(serialize_lock)); + add_assoc_long(return_value, "serialize_level", BG(serialize).level); + add_assoc_bool(return_value, "serialize_data", BG(serialize).data != NULL); + add_assoc_long(return_value, "unserialize_level", BG(unserialize).level); + add_assoc_bool(return_value, "unserialize_data", BG(unserialize).data != NULL); +} + /* Reallocate a block of old_size bytes to new_size bytes and return its block * size, along with the block size of a fresh allocation of new_size bytes. */ static ZEND_FUNCTION(zend_test_erealloc_block_size) diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index 8719c6935a33..62cc78cbe241 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -303,6 +303,10 @@ function zend_delref(mixed $variable): void {} function zend_test_erealloc_block_size(int $old_size, int $new_size): array {} + function zend_test_serialization_bailout(bool $unserialize, bool $locked): never {} + + function zend_test_serialization_state(): array {} + function zend_bool(bool $param): bool {} function zend_bool_or_null(bool|null $param): bool|null {} function zend_bool_slow_zpp(bool $param): bool {} diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index c2509001e45c..4d7bced1402d 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit test.stub.php instead. - * Stub hash: 55c444ca86bbbd773e43ddb6bd470efb371c45a4 + * Stub hash: a541ec4d43e8dfb243636d2517436196e337047e * Has decl header: yes */ #include "zend_attributes.h" @@ -68,6 +68,13 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_erealloc_block_size, 0 ZEND_ARG_TYPE_INFO(0, new_size, IS_LONG, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_serialization_bailout, 0, 2, IS_NEVER, 0) + ZEND_ARG_TYPE_INFO(0, unserialize, _IS_BOOL, 0) + ZEND_ARG_TYPE_INFO(0, locked, _IS_BOOL, 0) +ZEND_END_ARG_INFO() + +#define arginfo_zend_test_serialization_state arginfo_zend_test_array_return + ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_bool, 0, 1, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO(0, param, _IS_BOOL, 0) ZEND_END_ARG_INFO() @@ -473,6 +480,8 @@ static ZEND_FUNCTION(zend_leak_variable); static ZEND_FUNCTION(zend_leak_bytes); static ZEND_FUNCTION(zend_delref); static ZEND_FUNCTION(zend_test_erealloc_block_size); +static ZEND_FUNCTION(zend_test_serialization_bailout); +static ZEND_FUNCTION(zend_test_serialization_state); static ZEND_FUNCTION(zend_bool); static ZEND_FUNCTION(zend_bool_or_null); static ZEND_FUNCTION(zend_bool_slow_zpp); @@ -657,6 +666,8 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(zend_leak_bytes, arginfo_zend_leak_bytes) ZEND_FE(zend_delref, arginfo_zend_delref) ZEND_FE(zend_test_erealloc_block_size, arginfo_zend_test_erealloc_block_size) + ZEND_FE(zend_test_serialization_bailout, arginfo_zend_test_serialization_bailout) + ZEND_FE(zend_test_serialization_state, arginfo_zend_test_serialization_state) ZEND_FE(zend_bool, arginfo_zend_bool) ZEND_FE(zend_bool_or_null, arginfo_zend_bool_or_null) ZEND_FE(zend_bool_slow_zpp, arginfo_zend_bool_slow_zpp) diff --git a/ext/zend_test/test_decl.h b/ext/zend_test/test_decl.h index 291f7ab87afb..8fc579dd6879 100644 --- a/ext/zend_test/test_decl.h +++ b/ext/zend_test/test_decl.h @@ -1,8 +1,8 @@ /* This is a generated file, edit test.stub.php instead. - * Stub hash: 55c444ca86bbbd773e43ddb6bd470efb371c45a4 */ + * Stub hash: a541ec4d43e8dfb243636d2517436196e337047e */ -#ifndef ZEND_TEST_DECL_55c444ca86bbbd773e43ddb6bd470efb371c45a4_H -#define ZEND_TEST_DECL_55c444ca86bbbd773e43ddb6bd470efb371c45a4_H +#ifndef ZEND_TEST_DECL_a541ec4d43e8dfb243636d2517436196e337047e_H +#define ZEND_TEST_DECL_a541ec4d43e8dfb243636d2517436196e337047e_H typedef enum zend_enum_ZendTestUnitEnum { ZEND_ENUM_ZendTestUnitEnum_Foo = 1, @@ -27,4 +27,4 @@ typedef enum zend_enum_ZendTestEnumWithInterface { ZEND_ENUM_ZendTestEnumWithInterface_Bar = 2, } zend_enum_ZendTestEnumWithInterface; -#endif /* ZEND_TEST_DECL_55c444ca86bbbd773e43ddb6bd470efb371c45a4_H */ +#endif /* ZEND_TEST_DECL_a541ec4d43e8dfb243636d2517436196e337047e_H */ diff --git a/ext/zend_test/test_legacy_arginfo.h b/ext/zend_test/test_legacy_arginfo.h index d6e1f758e209..f654203e2a60 100644 --- a/ext/zend_test/test_legacy_arginfo.h +++ b/ext/zend_test/test_legacy_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit test.stub.php instead. - * Stub hash: 55c444ca86bbbd773e43ddb6bd470efb371c45a4 + * Stub hash: a541ec4d43e8dfb243636d2517436196e337047e * Has decl header: yes */ #include "zend_constants.h" @@ -63,6 +63,13 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_zend_test_erealloc_block_size, 0, 0, 2) ZEND_ARG_INFO(0, new_size) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_INFO_EX(arginfo_zend_test_serialization_bailout, 0, 0, 2) + ZEND_ARG_INFO(0, unserialize) + ZEND_ARG_INFO(0, locked) +ZEND_END_ARG_INFO() + +#define arginfo_zend_test_serialization_state arginfo_zend_trigger_bailout + ZEND_BEGIN_ARG_INFO_EX(arginfo_zend_bool, 0, 0, 1) ZEND_ARG_INFO(0, param) ZEND_END_ARG_INFO() @@ -397,6 +404,8 @@ static ZEND_FUNCTION(zend_leak_variable); static ZEND_FUNCTION(zend_leak_bytes); static ZEND_FUNCTION(zend_delref); static ZEND_FUNCTION(zend_test_erealloc_block_size); +static ZEND_FUNCTION(zend_test_serialization_bailout); +static ZEND_FUNCTION(zend_test_serialization_state); static ZEND_FUNCTION(zend_bool); static ZEND_FUNCTION(zend_bool_or_null); static ZEND_FUNCTION(zend_bool_slow_zpp); @@ -553,6 +562,8 @@ static const zend_function_entry ext_functions[] = { ZEND_FE(zend_leak_bytes, arginfo_zend_leak_bytes) ZEND_FE(zend_delref, arginfo_zend_delref) ZEND_FE(zend_test_erealloc_block_size, arginfo_zend_test_erealloc_block_size) + ZEND_FE(zend_test_serialization_bailout, arginfo_zend_test_serialization_bailout) + ZEND_FE(zend_test_serialization_state, arginfo_zend_test_serialization_state) ZEND_FE(zend_bool, arginfo_zend_bool) ZEND_FE(zend_bool_or_null, arginfo_zend_bool_or_null) ZEND_FE(zend_bool_slow_zpp, arginfo_zend_bool_slow_zpp) diff --git a/sapi/cli/tests/serialization_context_request_reset.phpt b/sapi/cli/tests/serialization_context_request_reset.phpt new file mode 100644 index 000000000000..caaf086ab179 --- /dev/null +++ b/sapi/cli/tests/serialization_context_request_reset.phpt @@ -0,0 +1,172 @@ +--TEST-- +Serialization contexts abandoned by a bailout are reset before processing the next request +--EXTENSIONS-- +session +zend_test +--INI-- +allow_url_fopen=1 +--ENV-- +PHP_CLI_SERVER_WORKERS= +--SKIPIF-- + +--FILE-- + ["nested" => 42]]; + session_write_close(); + echo "bailout\n"; + zend_test_serialization_bailout((bool) $_GET["unserialize"], (bool) $_GET["locked"]); +} +var_dump(zend_test_serialization_state()); +var_dump($_SESSION["value"]); +var_dump($_SESSION["upload_progress_test"]["done"]); +var_dump($_FILES["file"]["error"]); +session_destroy(); +PHP, cmd_args: $args); + +$body = "--boundary\r\n" + . "Content-Disposition: form-data; name=\"PHP_SESSION_UPLOAD_PROGRESS\"\r\n\r\n" + . "test\r\n" + . "--boundary\r\n" + . "Content-Disposition: form-data; name=\"file\"; filename=\"test.txt\"\r\n" + . "Content-Type: text/plain\r\n\r\n" + . "contents\r\n" + . "--boundary--\r\n"; + +foreach ([0, 1] as $unserialize) { + foreach ([0, 1] as $locked) { + [$pid, $response] = explode("\n", file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS + . "/?bailout=1&unserialize=$unserialize&locked=$locked"), 2); + echo $response; + $context = stream_context_create(["http" => [ + "method" => "POST", + "header" => "Content-Type: multipart/form-data; boundary=boundary\r\n" + . "Cookie: PHPSESSID=serialization-reset\r\n", + "content" => $body, + ]]); + $response = file_get_contents("http://" . PHP_CLI_SERVER_ADDRESS, false, $context); + if ($response === false) { + echo file_get_contents($server->outputFile); + break 2; + } + [$nextPid, $response] = explode("\n", $response, 2); + if ($nextPid !== $pid) { + throw new RuntimeException("Requests must use the same worker"); + } + echo $response; + } +} +?> +--EXPECT-- +bailout +array(5) { + ["lock"]=> + int(0) + ["serialize_level"]=> + int(0) + ["serialize_data"]=> + bool(false) + ["unserialize_level"]=> + int(0) + ["unserialize_data"]=> + bool(false) +} +array(1) { + ["nested"]=> + int(42) +} +bool(true) +int(0) +bailout +array(5) { + ["lock"]=> + int(0) + ["serialize_level"]=> + int(0) + ["serialize_data"]=> + bool(false) + ["unserialize_level"]=> + int(0) + ["unserialize_data"]=> + bool(false) +} +array(1) { + ["nested"]=> + int(42) +} +bool(true) +int(0) +bailout +array(5) { + ["lock"]=> + int(0) + ["serialize_level"]=> + int(0) + ["serialize_data"]=> + bool(false) + ["unserialize_level"]=> + int(0) + ["unserialize_data"]=> + bool(false) +} +array(1) { + ["nested"]=> + int(42) +} +bool(true) +int(0) +bailout +array(5) { + ["lock"]=> + int(0) + ["serialize_level"]=> + int(0) + ["serialize_data"]=> + bool(false) + ["unserialize_level"]=> + int(0) + ["unserialize_data"]=> + bool(false) +} +array(1) { + ["nested"]=> + int(42) +} +bool(true) +int(0)