From f1fedec930b0cff134f4b8803761fa28e57abece 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/standard/basic_functions.c | 4 - ext/standard/php_var.h | 3 + ext/standard/var.c | 8 + 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 +- main/main.c | 2 + .../serialization_context_request_reset.phpt | 172 ++++++++++++++++++ 11 files changed, 251 insertions(+), 10 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/standard/basic_functions.c b/ext/standard/basic_functions.c index 9a86030ba019..9e8ee6772fbb 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -390,10 +390,6 @@ PHP_RINIT_FUNCTION(basic) /* {{{ */ { memset(BG(strtok_table), 0, 256); - BG(serialize_lock) = 0; - memset(&BG(serialize), 0, sizeof(BG(serialize))); - memset(&BG(unserialize), 0, sizeof(BG(unserialize))); - BG(strtok_string) = NULL; BG(strtok_last) = NULL; BG(ctype_string) = NULL; diff --git a/ext/standard/php_var.h b/ext/standard/php_var.h index 681114da907e..1b2f16d4be3e 100644 --- a/ext/standard/php_var.h +++ b/ext/standard/php_var.h @@ -26,6 +26,9 @@ PHPAPI zend_result php_var_export_ex(zval *struc, int level, smart_str *buf); PHPAPI void php_debug_zval_dump(zval *struc, int level); +/* Reset before request input processing, never during serialization. */ +PHPAPI void php_var_serialization_reset(void); + typedef struct php_serialize_data *php_serialize_data_t; typedef struct php_unserialize_data *php_unserialize_data_t; diff --git a/ext/standard/var.c b/ext/standard/var.c index d788e23ecce7..f91ce626460b 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -1406,6 +1406,14 @@ PHPAPI void php_var_serialize(smart_str *buf, zval *struc, php_serialize_data_t } /* }}} */ +PHPAPI void php_var_serialization_reset(void) +{ + /* A bailout can abandon contexts that the request heap will reclaim. */ + BG(serialize_lock) = 0; + memset(&BG(serialize), 0, sizeof(BG(serialize))); + memset(&BG(unserialize), 0, sizeof(BG(unserialize))); +} + PHPAPI php_serialize_data_t php_var_serialize_init(void) { struct php_serialize_data *d; /* fprintf(stderr, "SERIALIZE_INIT == lock: %u, level: %u\n", BG(serialize_lock), BG(serialize).level); */ diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 4867f2f41c69..87bab3d7e6fc 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" @@ -200,6 +201,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 db5ca629ade2..8c418d7ff4f7 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -301,6 +301,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 87622c185cff..1a537b60744a 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: ea2674e5d5dbf60aa5efab31132064b52a07bddd + * Stub hash: 315971a5846587551a02b07edc3a1e360a7b6168 * Has decl header: yes */ #include "zend_attributes.h" @@ -64,6 +64,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() @@ -463,6 +470,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); @@ -645,6 +654,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 db7652512c83..af96924b3cbb 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: ea2674e5d5dbf60aa5efab31132064b52a07bddd */ + * Stub hash: 315971a5846587551a02b07edc3a1e360a7b6168 */ -#ifndef ZEND_TEST_DECL_ea2674e5d5dbf60aa5efab31132064b52a07bddd_H -#define ZEND_TEST_DECL_ea2674e5d5dbf60aa5efab31132064b52a07bddd_H +#ifndef ZEND_TEST_DECL_315971a5846587551a02b07edc3a1e360a7b6168_H +#define ZEND_TEST_DECL_315971a5846587551a02b07edc3a1e360a7b6168_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_ea2674e5d5dbf60aa5efab31132064b52a07bddd_H */ +#endif /* ZEND_TEST_DECL_315971a5846587551a02b07edc3a1e360a7b6168_H */ diff --git a/ext/zend_test/test_legacy_arginfo.h b/ext/zend_test/test_legacy_arginfo.h index a673b373142a..dd81502629fe 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: ea2674e5d5dbf60aa5efab31132064b52a07bddd + * Stub hash: 315971a5846587551a02b07edc3a1e360a7b6168 * Has decl header: yes */ #include "zend_constants.h" @@ -59,6 +59,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() @@ -387,6 +394,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); @@ -541,6 +550,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/main/main.c b/main/main.c index 1e4acfc8aa98..4a8aeb1a8fc3 100644 --- a/main/main.c +++ b/main/main.c @@ -1854,6 +1854,8 @@ zend_result php_request_startup(void) { zend_result retval = SUCCESS; + /* Input processing can use serialization before module RINIT. */ + php_var_serialization_reset(); zend_interned_strings_activate(); #ifdef HAVE_DTRACE 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)