From c4b678fa70d6e58010f5608da401dfc0610403b4 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 28 Jan 2025 19:45:52 +0100 Subject: [PATCH 01/58] PHP-8.3 is now for PHP 8.3.18-dev --- NEWS | 5 ++++- Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 6 +++--- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index e484b900923ed..6457d9080a76e 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.3.17 +?? ??? ????, PHP 8.3.18 + + +13 Feb 2025, PHP 8.3.17 - Core: . Fixed bug GH-16892 (ini_parse_quantity() fails to parse inputs starting diff --git a/Zend/zend.h b/Zend/zend.h index 0f73f888a6032..a3e833da7ef0b 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.3.17-dev" +#define ZEND_VERSION "4.3.18-dev" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index ee039a0bad50a..af4a1b3fc64dd 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.3.17-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.3.18-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index f980e0328e8c3..32d4dae3d7b21 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -2,7 +2,7 @@ /* edit configure.ac to change version number */ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 3 -#define PHP_RELEASE_VERSION 17 +#define PHP_RELEASE_VERSION 18 #define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.3.17-dev" -#define PHP_VERSION_ID 80317 +#define PHP_VERSION "8.3.18-dev" +#define PHP_VERSION_ID 80318 From ed1d51fa4fda4572982e390e7b0c8c9dc8d9f622 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Wed, 29 Jan 2025 09:46:15 +0100 Subject: [PATCH 02/58] relax test for zlib-ng --- ext/standard/tests/filters/gh13264.phpt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/standard/tests/filters/gh13264.phpt b/ext/standard/tests/filters/gh13264.phpt index 6456a082a1e40..e992d0868898d 100644 --- a/ext/standard/tests/filters/gh13264.phpt +++ b/ext/standard/tests/filters/gh13264.phpt @@ -45,5 +45,5 @@ array(4) { ["line"]=> int(%d) } -string(7) "Hello 6" +string(%d) "Hello%s" From 5a4832f97b2d195ab57990d9ff2ea1a8341dbcf4 Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Wed, 29 Jan 2025 19:01:00 +0900 Subject: [PATCH 03/58] Fixed GH-17398: bcmul memory leak (#17615) Changed BCG memory allocation to be forcibly released in PHP_GSHUTDOWN_FUNCTION regardless of refcount. Fixes #17398 Closes #17615 --- NEWS | 2 ++ ext/bcmath/bcmath.c | 6 +++--- ext/bcmath/libbcmath/src/bcmath.h | 2 ++ ext/bcmath/libbcmath/src/init.c | 7 +++++++ ext/bcmath/tests/gh17398.phpt | 10 ++++++++++ 5 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 ext/bcmath/tests/gh17398.phpt diff --git a/NEWS b/NEWS index 6457d9080a76e..0b3b9b5e26f02 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.18 +- BCMath: + . Fixed bug GH-17398 (bcmul memory leak). (SakiTakamachi) 13 Feb 2025, PHP 8.3.17 diff --git a/ext/bcmath/bcmath.c b/ext/bcmath/bcmath.c index f53032fabc35c..bc78faa7b3994 100644 --- a/ext/bcmath/bcmath.c +++ b/ext/bcmath/bcmath.c @@ -96,9 +96,9 @@ static PHP_GINIT_FUNCTION(bcmath) /* {{{ PHP_GSHUTDOWN_FUNCTION */ static PHP_GSHUTDOWN_FUNCTION(bcmath) { - _bc_free_num_ex(&bcmath_globals->_zero_, 1); - _bc_free_num_ex(&bcmath_globals->_one_, 1); - _bc_free_num_ex(&bcmath_globals->_two_, 1); + bc_force_free_number(&bcmath_globals->_zero_); + bc_force_free_number(&bcmath_globals->_one_); + bc_force_free_number(&bcmath_globals->_two_); } /* }}} */ diff --git a/ext/bcmath/libbcmath/src/bcmath.h b/ext/bcmath/libbcmath/src/bcmath.h index de51ee7457110..0ec86f2086199 100644 --- a/ext/bcmath/libbcmath/src/bcmath.h +++ b/ext/bcmath/libbcmath/src/bcmath.h @@ -87,6 +87,8 @@ typedef struct bc_struct { void bc_init_numbers(void); +void bc_force_free_number(bc_num *num); + bc_num _bc_new_num_ex(size_t length, size_t scale, bool persistent); void _bc_free_num_ex(bc_num *num, bool persistent); diff --git a/ext/bcmath/libbcmath/src/init.c b/ext/bcmath/libbcmath/src/init.c index cef55324689f9..6bb538ef1cb16 100644 --- a/ext/bcmath/libbcmath/src/init.c +++ b/ext/bcmath/libbcmath/src/init.c @@ -82,6 +82,13 @@ void bc_init_numbers(void) BCG(_two_)->n_value[0] = 2; } +void bc_force_free_number(bc_num *num) +{ + pefree((*num)->n_ptr, 1); + pefree(*num, 1); + *num = NULL; +} + /* Make a copy of a number! Just increments the reference count! */ bc_num bc_copy_num(bc_num num) diff --git a/ext/bcmath/tests/gh17398.phpt b/ext/bcmath/tests/gh17398.phpt new file mode 100644 index 0000000000000..6a0cda09ec76f --- /dev/null +++ b/ext/bcmath/tests/gh17398.phpt @@ -0,0 +1,10 @@ +--TEST-- +GH-17398 (bcmul memory leak) +--EXTENSIONS-- +bcmath +--FILE-- + +--EXPECTF-- +Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d From 333f5dd8489bf9efb4f318dba3c433b8158d07d1 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 29 Jan 2025 15:04:17 +0100 Subject: [PATCH 04/58] Fix stack overflow detection for variable compilation Closes GH-17623 --- NEWS | 4 ++ Zend/tests/stack_limit/stack_limit_015.phpt | 71 +++++++++++++++++++++ Zend/zend_compile.c | 4 ++ 3 files changed, 79 insertions(+) create mode 100644 Zend/tests/stack_limit/stack_limit_015.phpt diff --git a/NEWS b/NEWS index 0b3b9b5e26f02..036d611344452 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,10 @@ PHP NEWS - BCMath: . Fixed bug GH-17398 (bcmul memory leak). (SakiTakamachi) +- Core: + . Fixed bug GH-17623 (Broken stack overflow detection for variable + compilation). (ilutov) + 13 Feb 2025, PHP 8.3.17 - Core: diff --git a/Zend/tests/stack_limit/stack_limit_015.phpt b/Zend/tests/stack_limit/stack_limit_015.phpt new file mode 100644 index 0000000000000..b725523b7840a --- /dev/null +++ b/Zend/tests/stack_limit/stack_limit_015.phpt @@ -0,0 +1,71 @@ +--TEST-- +Stack limit 015 - Internal stack limit check in zend_compile_var() +--CREDITS-- +abdullahasif88 +--SKIPIF-- + +--EXTENSIONS-- +zend_test +--INI-- +zend.max_allowed_stack_size=128K +--FILE-- +p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p + ->p->p->p->p->p->p->p->p->p->p +; + +?> +--EXPECTF-- +Fatal error: Maximum call stack size of %d bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached during compilation. Try splitting expression in %s on line %d diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index dbd8c9dc17fd7..41113e2f0055b 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -10686,6 +10686,8 @@ static zend_op *zend_compile_var_inner(znode *result, zend_ast *ast, uint32_t ty static zend_op *zend_compile_var(znode *result, zend_ast *ast, uint32_t type, bool by_ref) /* {{{ */ { + zend_check_stack_limit(); + uint32_t checkpoint = zend_short_circuiting_checkpoint(); zend_op *opcode = zend_compile_var_inner(result, ast, type, by_ref); zend_short_circuiting_commit(checkpoint, result, ast); @@ -10694,6 +10696,8 @@ static zend_op *zend_compile_var(znode *result, zend_ast *ast, uint32_t type, bo static zend_op *zend_delayed_compile_var(znode *result, zend_ast *ast, uint32_t type, bool by_ref) /* {{{ */ { + zend_check_stack_limit(); + switch (ast->kind) { case ZEND_AST_VAR: return zend_compile_simple_var(result, ast, type, 1); From 5447473785b9410bf895c36a6ace065bdab55d7a Mon Sep 17 00:00:00 2001 From: ndossche Date: Thu, 30 Jan 2025 10:29:46 +0100 Subject: [PATCH 05/58] Partially fix GH-17387 The length of the string should be set to the truncated length (that was used to duplicate the input anyway). --- NEWS | 3 +++ sapi/phpdbg/phpdbg_lexer.l | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 036d611344452..7c51ecc121c85 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,9 @@ PHP NEWS . Fixed bug GH-17623 (Broken stack overflow detection for variable compilation). (ilutov) +- PHPDBG: + . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) + 13 Feb 2025, PHP 8.3.17 - Core: diff --git a/sapi/phpdbg/phpdbg_lexer.l b/sapi/phpdbg/phpdbg_lexer.l index 6245262a00598..60d995526ea27 100644 --- a/sapi/phpdbg/phpdbg_lexer.l +++ b/sapi/phpdbg/phpdbg_lexer.l @@ -160,8 +160,9 @@ INPUT ("\\"[#"']|["]("\\\\"|"\\"["]|[^\n\000"])*["]|[']("\\"[']|"\\\\"|[^\ {GENERIC_ID} { phpdbg_init_param(yylval, STR_PARAM); - yylval->str = estrndup(yytext, yyleng - unescape_string(yytext)); - yylval->len = yyleng; + size_t len = yyleng - unescape_string(yytext); + yylval->str = estrndup(yytext, len); + yylval->len = len; return T_ID; } From 62bbfdebaa0458eee31bb015b507f36653fcdf5c Mon Sep 17 00:00:00 2001 From: ndossche Date: Thu, 30 Jan 2025 10:30:25 +0100 Subject: [PATCH 06/58] Fix memory leak in phpdbg calling registered function Closes GH-17635. --- NEWS | 1 + sapi/phpdbg/phpdbg_prompt.c | 3 +++ sapi/phpdbg/tests/register_function_leak.phpt | 24 +++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 sapi/phpdbg/tests/register_function_leak.phpt diff --git a/NEWS b/NEWS index 7c51ecc121c85..ca459617aafb5 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,7 @@ PHP NEWS - PHPDBG: . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) + . Fix memory leak in phpdbg calling registered function. (nielsdos) 13 Feb 2025, PHP 8.3.17 diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c index 5276d62ee295e..e43bf647c7611 100644 --- a/sapi/phpdbg/phpdbg_prompt.c +++ b/sapi/phpdbg/phpdbg_prompt.c @@ -189,6 +189,9 @@ static inline int phpdbg_call_register(phpdbg_param_t *stack) /* {{{ */ zval_ptr_dtor_str(&fci.function_name); efree(lc_name); + if (fci.named_params) { + zend_array_destroy(fci.named_params); + } return SUCCESS; } diff --git a/sapi/phpdbg/tests/register_function_leak.phpt b/sapi/phpdbg/tests/register_function_leak.phpt new file mode 100644 index 0000000000000..b5416ea95bcc8 --- /dev/null +++ b/sapi/phpdbg/tests/register_function_leak.phpt @@ -0,0 +1,24 @@ +--TEST-- +registering a function and calling it leaks arguments memory +--FILE-- + +--PHPDBG-- +register var_dump +var_dump "a" "b" +register flush +flush +r +q +--EXPECTF-- +[Successful compilation of %s] +prompt> [Registered var_dump] +prompt> string(1) "a" +string(1) "b" + +prompt> [Registered flush] +prompt> +prompt> Done +[Script ended normally] +prompt> From f8b57ff1bf89537946172a3ee58dd2714913835e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 31 Jan 2025 10:19:49 +0100 Subject: [PATCH 07/58] zend_execute: Suppress values in `UnhandledMatchError` for `zend.exception_ignore_args=1` (#17619) Fixes php/php-src#17618. --- NEWS | 2 ++ Zend/tests/match/049.phpt | 42 +++++++++++++++++++++++++++++++++++++++ Zend/zend_execute.c | 2 +- 3 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/match/049.phpt diff --git a/NEWS b/NEWS index ca459617aafb5..8469bdc9df857 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,8 @@ PHP NEWS - Core: . Fixed bug GH-17623 (Broken stack overflow detection for variable compilation). (ilutov) + . Fixed bug GH-17618 (UnhandledMatchError does not take + zend.exception_ignore_args=1 into account). (timwolla) - PHPDBG: . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) diff --git a/Zend/tests/match/049.phpt b/Zend/tests/match/049.phpt new file mode 100644 index 0000000000000..2a2385f704ff4 --- /dev/null +++ b/Zend/tests/match/049.phpt @@ -0,0 +1,42 @@ +--TEST-- +Match expression error messages (zend.exception_ignore_args=1) +--INI-- +zend.exception_ignore_args=1 +--FILE-- +getMessage() . PHP_EOL; + } +} + +test(null); +test(1); +test(5.5); +test(5.0); +test("foo"); +test(true); +test(false); +test([1, 2, 3]); +test(new Beep()); +// Testing long strings. +test(str_repeat('e', 100)); +test(str_repeat("e\n", 100)); +?> +--EXPECT-- +Unhandled match case of type null +Unhandled match case of type int +Unhandled match case of type float +Unhandled match case of type float +Unhandled match case of type string +Unhandled match case of type bool +Unhandled match case of type bool +Unhandled match case of type array +Unhandled match case of type Beep +Unhandled match case of type string +Unhandled match case of type string diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index bb3991f695f8b..dc28578f2c1a3 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -872,7 +872,7 @@ ZEND_COLD void zend_match_unhandled_error(const zval *value) { smart_str msg = {0}; - if (Z_TYPE_P(value) <= IS_STRING) { + if (!EG(exception_ignore_args) && Z_TYPE_P(value) <= IS_STRING) { smart_str_append_scalar(&msg, value, EG(exception_string_param_max_len)); } else { smart_str_appendl(&msg, "of type ", sizeof("of type ")-1); From afe8e2cdff73fc92549381324151e9e7acdeba9b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Sun, 2 Feb 2025 11:21:45 +0100 Subject: [PATCH 08/58] Relax timezone_IDforWindowsID_basic2.phpt expectations Apparently, some ICU versions report "America/Los_Angeles" for the `ZZ` case, what matches the behavior of ICU 76.1 (on Windows). Possibly, there has been some bug fix backport on some systems. Anyhow, either seems fine, so we're not picky about that. Closes GH-17669. --- ext/intl/tests/timezone_IDforWindowsID_basic2.phpt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/intl/tests/timezone_IDforWindowsID_basic2.phpt b/ext/intl/tests/timezone_IDforWindowsID_basic2.phpt index aeb9b16899157..2b8f223dad807 100644 --- a/ext/intl/tests/timezone_IDforWindowsID_basic2.phpt +++ b/ext/intl/tests/timezone_IDforWindowsID_basic2.phpt @@ -24,7 +24,7 @@ foreach ($tzs as $tz => $regions) { } } ?> ---EXPECT-- +--EXPECTF-- ** Gnomeregan bool(false) Error: intltz_get_windows_id: Unknown windows timezone: U_ILLEGAL_ARGUMENT_ERROR @@ -35,7 +35,7 @@ string(19) "America/Los_Angeles" string(17) "America/Vancouver" string(19) "America/Los_Angeles" string(19) "America/Los_Angeles" -string(7) "PST8PDT" +string(%d) "%r(PST8PDT|America\/Los_Angeles)%r" ** Romance Standard Time string(12) "Europe/Paris" string(15) "Europe/Brussels" From f88445bdf894b2f0b3f4efcf5176de3cc33c4350 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 31 Jan 2025 21:41:45 +0100 Subject: [PATCH 09/58] Fix GH-17654: Multiple classes using same trait causes function JIT crash This test has two classes that use the same trait. In function JIT mode the same cache slot will be used. This causes problems because it is primed for the first class and then reused for the second class, resulting in an incorrect type check failure. The current check for a megamorphic trait call requires current_frame to not be NULL, but this is only set in tracing mode and not in function mode. This patch corrects the check. Closes GH-17660. --- NEWS | 4 +++ ext/opcache/jit/zend_jit_arm64.dasc | 6 ++--- ext/opcache/jit/zend_jit_x86.dasc | 6 ++--- ext/opcache/tests/jit/gh17654.phpt | 38 +++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 ext/opcache/tests/jit/gh17654.phpt diff --git a/NEWS b/NEWS index 8469bdc9df857..aace72eb49f00 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,10 @@ PHP NEWS . Fixed bug GH-17618 (UnhandledMatchError does not take zend.exception_ignore_args=1 into account). (timwolla) +- Opcache: + . Fixed bug GH-17654 (Multiple classes using same trait causes function + JIT crash). (nielsdos) + - PHPDBG: . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) . Fix memory leak in phpdbg calling registered function. (nielsdos) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 2cbf68643086a..ec6fae8819fcd 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -9200,9 +9200,9 @@ static int zend_jit_do_fcall(dasm_State **Dst, const zend_op *opline, const zend func = call_info->callee_func; } if ((op_array->fn_flags & ZEND_ACC_TRAIT_CLONE) - && JIT_G(current_frame) - && JIT_G(current_frame)->call - && !JIT_G(current_frame)->call->func) { + && (!JIT_G(current_frame) || + !JIT_G(current_frame)->call || + !JIT_G(current_frame)->call->func)) { call_info = NULL; func = NULL; /* megamorphic call from trait */ } } diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 9cf0c6cd8e881..f65dc769db57d 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -9931,9 +9931,9 @@ static int zend_jit_do_fcall(dasm_State **Dst, const zend_op *opline, const zend func = call_info->callee_func; } if ((op_array->fn_flags & ZEND_ACC_TRAIT_CLONE) - && JIT_G(current_frame) - && JIT_G(current_frame)->call - && !JIT_G(current_frame)->call->func) { + && (!JIT_G(current_frame) || + !JIT_G(current_frame)->call || + !JIT_G(current_frame)->call->func)) { call_info = NULL; func = NULL; /* megamorphic call from trait */ } } diff --git a/ext/opcache/tests/jit/gh17654.phpt b/ext/opcache/tests/jit/gh17654.phpt new file mode 100644 index 0000000000000..59d9205b37f2a --- /dev/null +++ b/ext/opcache/tests/jit/gh17654.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-17654 (Multiple classes using same trait causes function JIT crash) +--EXTENSIONS-- +opcache +--INI-- +opcache.jit=1214 +opcache.jit_buffer_size=16M +--FILE-- +addUnit("test2"); + (new Test)->addUnit("test"); +} + +main(); +?> +--EXPECT-- +string(5) "test2" +string(4) "test" From 0c3cf1f311402126daaddeb8b86c5af1b2ce4204 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 26 Jan 2025 13:33:40 +0100 Subject: [PATCH 10/58] Fix GH-17577: JIT packed type guard crash When a guard check is created for a variable to check if it's a packed array, it is possible that there was no prior type check for that variable. This happens in the global scope for example when the variable aliases. In the test, this causes a dereference of address 8 because the integer element in `$a` is interpreted as an array address. This patch adds a check to see if the guard is handled. If we were not able to determine or guard the type then we also cannot know the array is packed. Closes GH-17584. --- NEWS | 1 + ext/opcache/jit/zend_jit_trace.c | 21 +++++++++++++++------ ext/opcache/tests/jit/gh17577.phpt | 27 +++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 ext/opcache/tests/jit/gh17577.phpt diff --git a/NEWS b/NEWS index aace72eb49f00..49ecf9f3de08b 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,7 @@ PHP NEWS - Opcache: . Fixed bug GH-17654 (Multiple classes using same trait causes function JIT crash). (nielsdos) + . Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry) - PHPDBG: . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index 5d824c207d18f..33c554a3f78ef 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -1745,7 +1745,8 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin if (!(orig_op1_type & IS_TRACE_PACKED)) { zend_ssa_var_info *info = &tssa->var_info[tssa->ops[idx].op1_use]; - if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type)) { + if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type) + && (info->type & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY) { info->type |= MAY_BE_PACKED_GUARD; info->type &= ~MAY_BE_ARRAY_PACKED; } @@ -1754,7 +1755,8 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin && val_type != IS_UNDEF) { zend_ssa_var_info *info = &tssa->var_info[tssa->ops[idx].op1_use]; - if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type)) { + if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type) + && (info->type & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY) { info->type |= MAY_BE_PACKED_GUARD; info->type &= ~(MAY_BE_ARRAY_NUMERIC_HASH|MAY_BE_ARRAY_STRING_HASH); } @@ -1833,7 +1835,8 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin zend_ssa_var_info *info = &tssa->var_info[tssa->ops[idx].op1_use]; - if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type)) { + if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type) + && (info->type & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY) { info->type |= MAY_BE_PACKED_GUARD; if (orig_op1_type & IS_TRACE_PACKED) { info->type &= ~(MAY_BE_ARRAY_NUMERIC_HASH|MAY_BE_ARRAY_STRING_HASH); @@ -1935,7 +1938,8 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin zend_ssa_var_info *info = &tssa->var_info[tssa->ops[idx].op1_use]; - if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type)) { + if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type) + && (info->type & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY) { info->type |= MAY_BE_PACKED_GUARD; if (orig_op1_type & IS_TRACE_PACKED) { info->type &= ~(MAY_BE_ARRAY_NUMERIC_HASH|MAY_BE_ARRAY_STRING_HASH); @@ -1965,7 +1969,8 @@ static zend_ssa *zend_jit_trace_build_tssa(zend_jit_trace_rec *trace_buffer, uin zend_ssa_var_info *info = &tssa->var_info[tssa->ops[idx].op1_use]; - if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type)) { + if (MAY_BE_PACKED(info->type) && MAY_BE_HASH(info->type) + && (info->type & (MAY_BE_ANY|MAY_BE_UNDEF)) == MAY_BE_ARRAY) { info->type |= MAY_BE_PACKED_GUARD; info->type &= ~MAY_BE_ARRAY_PACKED; } @@ -4164,10 +4169,14 @@ static const void *zend_jit_trace(zend_jit_trace_rec *trace_buffer, uint32_t par if ((info & MAY_BE_PACKED_GUARD) != 0 && (trace_buffer->stop == ZEND_JIT_TRACE_STOP_LOOP || trace_buffer->stop == ZEND_JIT_TRACE_STOP_RECURSIVE_CALL - || trace_buffer->stop == ZEND_JIT_TRACE_STOP_RECURSIVE_RET) + || (trace_buffer->stop == ZEND_JIT_TRACE_STOP_RECURSIVE_RET + && (opline-1)->result_type == IS_VAR + && EX_VAR_TO_NUM((opline-1)->result.var) == i)) && (ssa->vars[i].use_chain != -1 || (ssa->vars[i].phi_use_chain && !(ssa->var_info[ssa->vars[i].phi_use_chain->ssa_var].type & MAY_BE_PACKED_GUARD)))) { + ZEND_ASSERT(STACK_TYPE(stack, i) == IS_ARRAY); + if (!zend_jit_packed_guard(&dasm_state, opline, EX_NUM_TO_VAR(i), info)) { goto jit_failure; } diff --git a/ext/opcache/tests/jit/gh17577.phpt b/ext/opcache/tests/jit/gh17577.phpt new file mode 100644 index 0000000000000..2eac2d05e432d --- /dev/null +++ b/ext/opcache/tests/jit/gh17577.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-17577 (JIT packed type guard crash) +--EXTENSIONS-- +opcache +--INI-- +opcache.jit_buffer_size=16M +opcache.jit_hot_func=1 +--FILE-- + +--EXPECTF-- +Warning: Trying to access array offset on int in %s on line %d + +Warning: Trying to access array offset on int in %s on line %d + +Warning: Trying to access array offset on int in %s on line %d From 7e06a81bbd09100e50273a235c38fa8853dae5c9 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 2 Feb 2025 00:59:08 +0100 Subject: [PATCH 11/58] Fix fallback paths in fast_long_{add,sub}_function This was asked to be checked in https://github.com/php/php-src/pull/17472#issuecomment-2591325036 There are 2 issues: 1) The UB in the if can overflow, and can be fixed by using zend_ulong for the sum/sub. 2) fast_long_sub_function() has a problem when result aliases. This is fixed in the same way as fast_long_add_function() works. Closes GH-17666. --- NEWS | 1 + Zend/zend_operators.h | 18 ++++++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 49ecf9f3de08b..963a20eedce06 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,7 @@ PHP NEWS compilation). (ilutov) . Fixed bug GH-17618 (UnhandledMatchError does not take zend.exception_ignore_args=1 into account). (timwolla) + . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos) - Opcache: . Fixed bug GH-17654 (Multiple classes using same trait causes function diff --git a/Zend/zend_operators.h b/Zend/zend_operators.h index fd1db6f406328..76e95a92d4166 100644 --- a/Zend/zend_operators.h +++ b/Zend/zend_operators.h @@ -722,11 +722,13 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL * have read the values of op1 and op2. */ + zend_long sum = (zend_long) ((zend_ulong) Z_LVAL_P(op1) + (zend_ulong) Z_LVAL_P(op2)); + if (UNEXPECTED((Z_LVAL_P(op1) & LONG_SIGN_MASK) == (Z_LVAL_P(op2) & LONG_SIGN_MASK) - && (Z_LVAL_P(op1) & LONG_SIGN_MASK) != ((Z_LVAL_P(op1) + Z_LVAL_P(op2)) & LONG_SIGN_MASK))) { + && (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (sum & LONG_SIGN_MASK))) { ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) + (double) Z_LVAL_P(op2)); } else { - ZVAL_LONG(result, Z_LVAL_P(op1) + Z_LVAL_P(op2)); + ZVAL_LONG(result, sum); } #endif } @@ -804,11 +806,19 @@ overflow: ZEND_ATTRIBUTE_COLD_LABEL ZVAL_LONG(result, llresult); } #else - ZVAL_LONG(result, Z_LVAL_P(op1) - Z_LVAL_P(op2)); + /* + * 'result' may alias with op1 or op2, so we need to + * ensure that 'result' is not updated until after we + * have read the values of op1 and op2. + */ + + zend_long sub = (zend_long) ((zend_ulong) Z_LVAL_P(op1) - (zend_ulong) Z_LVAL_P(op2)); if (UNEXPECTED((Z_LVAL_P(op1) & LONG_SIGN_MASK) != (Z_LVAL_P(op2) & LONG_SIGN_MASK) - && (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (Z_LVAL_P(result) & LONG_SIGN_MASK))) { + && (Z_LVAL_P(op1) & LONG_SIGN_MASK) != (sub & LONG_SIGN_MASK))) { ZVAL_DOUBLE(result, (double) Z_LVAL_P(op1) - (double) Z_LVAL_P(op2)); + } else { + ZVAL_LONG(result, sub); } #endif } From d8aedb589c2de5f254c46f9da0dfdbc11433ec20 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 4 Feb 2025 14:51:33 +0100 Subject: [PATCH 12/58] [skip ci] Another flaky phar macOS test --- ext/phar/tests/zip/033.phpt | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/phar/tests/zip/033.phpt b/ext/phar/tests/zip/033.phpt index de4ba2b71f027..cf5ccd9a51050 100644 --- a/ext/phar/tests/zip/033.phpt +++ b/ext/phar/tests/zip/033.phpt @@ -5,6 +5,12 @@ phar --INI-- phar.readonly=0 phar.require_hash=0 +--SKIPIF-- + --FILE-- Date: Tue, 4 Feb 2025 12:13:24 +0100 Subject: [PATCH 13/58] Fix GH-17503: Undefined float conversion in mb_convert_variables Conversion of floating point to integer values is undefined if the integral part of the float value cannot be represented by the integer type. We need to cater to that explicitly (in a manner similar to `zend_dval_to_lval_cap()`). Closes GH-17689. --- NEWS | 4 ++++ ext/mbstring/mbstring.c | 3 ++- ext/mbstring/tests/gh17503.phpt | 11 +++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 ext/mbstring/tests/gh17503.phpt diff --git a/NEWS b/NEWS index 963a20eedce06..779a2e05523e6 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,10 @@ PHP NEWS zend.exception_ignore_args=1 into account). (timwolla) . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos) +- MBString: + . Fixed bug GH-17503 (Undefined float conversion in mb_convert_variables). + (cmb) + - Opcache: . Fixed bug GH-17654 (Multiple classes using same trait causes function JIT crash). (nielsdos) diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index 2da957454a902..dbf012174c494 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -3092,7 +3092,8 @@ try_next_encoding:; } for (size_t i = 0; i < length; i++) { - array[i].demerits *= array[i].multiplier; + double demerits = array[i].demerits * (double) array[i].multiplier; + array[i].demerits = demerits < (double) UINT64_MAX ? (uint64_t) demerits : UINT64_MAX; } return length; diff --git a/ext/mbstring/tests/gh17503.phpt b/ext/mbstring/tests/gh17503.phpt new file mode 100644 index 0000000000000..92a2cf39cb10f --- /dev/null +++ b/ext/mbstring/tests/gh17503.phpt @@ -0,0 +1,11 @@ +--TEST-- +GH-17503 (Undefined float conversion in mb_convert_variables) +--EXTENSIONS-- +mbstring +--FILE-- +"); +var_dump(mb_convert_variables("ASCII", ["UTF-8", "UTF-16"], $a)); +?> +--EXPECT-- +string(5) "UTF-8" From 36d46a473217929b38b8a7be805f2cb17a68dd85 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Wed, 5 Feb 2025 15:58:25 +0100 Subject: [PATCH 14/58] Fix curl_basic_022.phpt for libcurl 8.12.0 Due to a deliberate change in libcurl, the expiration is now capped to at most 400 days. We could solve this by choosing another date roughly a year in the future, but would need to update the test next year. This would be especially annoying for security branches. Another option would be to actually parse the cookie list lines, but that might not be worth the trouble. Instead we just ignore the exact timestamp created by libcurl. [1] Closes GH-17709. --- ext/curl/tests/curl_basic_022.phpt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/curl/tests/curl_basic_022.phpt b/ext/curl/tests/curl_basic_022.phpt index e905dfd885d16..4a2177e06bd47 100644 --- a/ext/curl/tests/curl_basic_022.phpt +++ b/ext/curl/tests/curl_basic_022.phpt @@ -11,10 +11,10 @@ curl_setopt($ch, CURLOPT_COOKIELIST, 'Set-Cookie: C2=v2; expires=Thu, 31-Dec-203 var_dump(curl_getinfo($ch, CURLINFO_COOKIELIST)); ?> ---EXPECT-- +--EXPECTF-- array(2) { [0]=> - string(38) ".php.net TRUE / FALSE 2145916799 C1 v1" + string(38) ".php.net TRUE / FALSE %d C1 v1" [1]=> - string(38) ".php.net TRUE / FALSE 2145916799 C2 v2" + string(38) ".php.net TRUE / FALSE %d C2 v2" } From 726cf51236b03c3922544f1ac521bc1fa9c2643e Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 5 Feb 2025 13:16:18 +0100 Subject: [PATCH 15/58] Add CONFLICT all to random port test If we're very unlucky, we can get the same port opened as an ephemeral port by some other test. Closes GH-17706 --- ext/standard/tests/network/bug20134.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/standard/tests/network/bug20134.phpt b/ext/standard/tests/network/bug20134.phpt index b5a7234f73c4f..6692e92b31052 100644 --- a/ext/standard/tests/network/bug20134.phpt +++ b/ext/standard/tests/network/bug20134.phpt @@ -2,6 +2,8 @@ Bug #20134 (UDP reads from invalid ports) --INI-- default_socket_timeout=1 +--CONFLICTS-- +all --FILE-- Date: Wed, 5 Feb 2025 19:45:09 +0100 Subject: [PATCH 16/58] Fix GH-17704: ldap_search fails when $attributes contains a non-packed array with numerical keys Closes GH-17710. --- NEWS | 4 ++++ ext/ldap/ldap.c | 2 +- ext/ldap/tests/gh17704.phpt | 24 ++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 ext/ldap/tests/gh17704.phpt diff --git a/NEWS b/NEWS index 779a2e05523e6..d2508bf5bde29 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,10 @@ PHP NEWS zend.exception_ignore_args=1 into account). (timwolla) . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos) +- LDAP: + . Fixed bug GH-17704 (ldap_search fails when $attributes contains a + non-packed array with numerical keys). (nielsdos, 7u83) + - MBString: . Fixed bug GH-17503 (Undefined float conversion in mb_convert_variables). (cmb) diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index e66ff070577bd..db4c5ab231042 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -248,7 +248,7 @@ static bool php_ldap_is_numerically_indexed_array(zend_array *arr) } } ZEND_HASH_FOREACH_END(); - return false; + return true; } /* {{{ Parse controls from and to arrays */ diff --git a/ext/ldap/tests/gh17704.phpt b/ext/ldap/tests/gh17704.phpt new file mode 100644 index 0000000000000..2403a63860e4a --- /dev/null +++ b/ext/ldap/tests/gh17704.phpt @@ -0,0 +1,24 @@ +--TEST-- +GH-17704 (ldap_search fails when $attributes contains a non-packed array with numerical keys) +--EXTENSIONS-- +ldap +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +object(LDAP\Result)#%d (0) { +} From 0607b663d39907093c2fc73779d55f8422e9eef9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 7 Feb 2025 09:36:33 +0100 Subject: [PATCH 17/58] Disallow calls to abstract `__call()` / `__callStatic()` (#17719) Fixes php/php-src#17718 --- NEWS | 2 ++ Zend/tests/gh_17718_001.phpt | 17 +++++++++++++++++ Zend/tests/gh_17718_002.phpt | 17 +++++++++++++++++ Zend/zend_object_handlers.c | 6 +++++- 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/gh_17718_001.phpt create mode 100644 Zend/tests/gh_17718_002.phpt diff --git a/NEWS b/NEWS index d2508bf5bde29..3fed1c61baf44 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,8 @@ PHP NEWS . Fixed bug GH-17618 (UnhandledMatchError does not take zend.exception_ignore_args=1 into account). (timwolla) . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos) + . Fixed bug GH-17718 (Calling static methods on an interface that has + `__callStatic` is allowed). (timwolla) - LDAP: . Fixed bug GH-17704 (ldap_search fails when $attributes contains a diff --git a/Zend/tests/gh_17718_001.phpt b/Zend/tests/gh_17718_001.phpt new file mode 100644 index 0000000000000..1f5bee961eb71 --- /dev/null +++ b/Zend/tests/gh_17718_001.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-17718: Disallow calling abstract `__callStatic()` trampoline on an interface +--FILE-- + +--EXPECTF-- +Fatal error: Uncaught Error: Cannot call abstract method Foo::bar() in %s:%d +Stack trace: +#0 {main} + thrown in %s on line %d diff --git a/Zend/tests/gh_17718_002.phpt b/Zend/tests/gh_17718_002.phpt new file mode 100644 index 0000000000000..f3f75fca40f72 --- /dev/null +++ b/Zend/tests/gh_17718_002.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-17718: Disallow calling abstract `__callStatic()` trampoline on an abstract class +--FILE-- + +--EXPECTF-- +Fatal error: Uncaught Error: Cannot call abstract method Foo::bar() in %s:%d +Stack trace: +#0 {main} + thrown in %s on line %d diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 10664cd118118..2576fda51ec3d 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1348,7 +1348,7 @@ ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce func->fn_flags = ZEND_ACC_CALL_VIA_TRAMPOLINE | ZEND_ACC_PUBLIC | ZEND_ACC_VARIADIC - | (fbc->common.fn_flags & ZEND_ACC_RETURN_REFERENCE); + | (fbc->common.fn_flags & (ZEND_ACC_RETURN_REFERENCE|ZEND_ACC_ABSTRACT)); if (is_static) { func->fn_flags |= ZEND_ACC_STATIC; } @@ -1541,6 +1541,10 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, zend_st if (EXPECTED(fbc)) { if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_ABSTRACT)) { zend_abstract_method_call(fbc); + if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) { + zend_string_release_ex(fbc->common.function_name, 0); + zend_free_trampoline(fbc); + } fbc = NULL; } else if (UNEXPECTED(fbc->common.scope->ce_flags & ZEND_ACC_TRAIT)) { zend_error(E_DEPRECATED, From 00d4390ea1754909e16331cb9aa72d7d04a7258a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 7 Feb 2025 10:53:14 +0100 Subject: [PATCH 18/58] Free the trampoline when deprecation on materializing `__callStatic()` of trait throws (#17729) Fixes php/php-src#17728 --- Zend/tests/gh_17728.phpt | 24 ++++++++++++++++++++++++ Zend/zend_object_handlers.c | 16 ++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 Zend/tests/gh_17728.phpt diff --git a/Zend/tests/gh_17728.phpt b/Zend/tests/gh_17728.phpt new file mode 100644 index 0000000000000..ef458a8e8d5e1 --- /dev/null +++ b/Zend/tests/gh_17728.phpt @@ -0,0 +1,24 @@ +--TEST-- +GH-17728: Assertion failure when calling static method of trait with `__callStatic()` with throwing error handler +--FILE-- +getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +Calling static trait method Foo::bar is deprecated, it should only be called on a class using the trait diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 2576fda51ec3d..d688e4b63ed69 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1541,23 +1541,27 @@ ZEND_API zend_function *zend_std_get_static_method(zend_class_entry *ce, zend_st if (EXPECTED(fbc)) { if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_ABSTRACT)) { zend_abstract_method_call(fbc); - if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) { - zend_string_release_ex(fbc->common.function_name, 0); - zend_free_trampoline(fbc); - } - fbc = NULL; + goto fail; } else if (UNEXPECTED(fbc->common.scope->ce_flags & ZEND_ACC_TRAIT)) { zend_error(E_DEPRECATED, "Calling static trait method %s::%s is deprecated, " "it should only be called on a class using the trait", ZSTR_VAL(fbc->common.scope->name), ZSTR_VAL(fbc->common.function_name)); if (EG(exception)) { - return NULL; + goto fail; } } } return fbc; + + fail: + if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) { + zend_string_release_ex(fbc->common.function_name, 0); + zend_free_trampoline(fbc); + } + + return NULL; } /* }}} */ From fd5d6ad5bdcc3034a6ead9a0afe639e62d364965 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 31 Jan 2025 20:03:25 +0100 Subject: [PATCH 19/58] Fix GH-17650: realloc with size 0 in user_filters.c If the returned buffer string is of length 0, then a realloc can happen with length 0. However, the behaviour is implementation-defined. From 7.20.3.1 of C11 spec: > If the size of the space requested is zero, the behavior is > implementation-defined: either a null pointer is returned, > or the behavior is as if the size were some nonzero value, > except that the returned pointer shall not be used to access an object This is problematic for the test case on my system as it returns NULL, causing a memleak and later using it in memcpy causing UB. The bucket code is not prepared to handle a NULL pointer. To solve this, we use MAX to clamp the size to 1 at the least. Closes GH-17656. --- NEWS | 3 +++ ext/standard/tests/streams/gh17650.phpt | 35 +++++++++++++++++++++++++ ext/standard/user_filters.c | 2 +- 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 ext/standard/tests/streams/gh17650.phpt diff --git a/NEWS b/NEWS index 3fed1c61baf44..9d327f6bb951d 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,9 @@ PHP NEWS . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) . Fix memory leak in phpdbg calling registered function. (nielsdos) +- Streams: + . Fixed bug GH-17650 (realloc with size 0 in user_filters.c). (nielsdos) + 13 Feb 2025, PHP 8.3.17 - Core: diff --git a/ext/standard/tests/streams/gh17650.phpt b/ext/standard/tests/streams/gh17650.phpt new file mode 100644 index 0000000000000..516478b5d66dd --- /dev/null +++ b/ext/standard/tests/streams/gh17650.phpt @@ -0,0 +1,35 @@ +--TEST-- +GH-17650 (realloc with size 0 in user_filters.c) +--FILE-- +data = ''; + $consumed += strlen($bucket->data); + stream_bucket_append($out, $bucket); + } + return PSFS_PASS_ON; + } +} + +stream_filter_register('testfilter','testfilter'); + +$text = "Hello There!"; + +$fp = fopen('php://memory', 'w+'); +fwrite($fp, $text); + +rewind($fp); +stream_filter_append($fp, 'testfilter', STREAM_FILTER_READ, 'testuserfilter'); + +while ($x = fgets($fp)) { + var_dump($x); +} + +fclose($fp); + +echo "Done\n"; +?> +--EXPECT-- +Done diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index 737237f6630cd..acef5146fa25e 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -398,7 +398,7 @@ static void php_stream_bucket_attach(int append, INTERNAL_FUNCTION_PARAMETERS) bucket = php_stream_bucket_make_writeable(bucket); } if (bucket->buflen != Z_STRLEN_P(pzdata)) { - bucket->buf = perealloc(bucket->buf, Z_STRLEN_P(pzdata), bucket->is_persistent); + bucket->buf = perealloc(bucket->buf, MAX(Z_STRLEN_P(pzdata), 1), bucket->is_persistent); bucket->buflen = Z_STRLEN_P(pzdata); } memcpy(bucket->buf, Z_STRVAL_P(pzdata), bucket->buflen); From e48ceb0026c41f39faab658446290c58aeb9b06b Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 30 Jan 2025 19:03:19 +0100 Subject: [PATCH 20/58] Fix GH-17643: FPM with httpd ProxyPass encoded PATH_INFO env Closes GH-17644 --- NEWS | 4 ++++ sapi/fpm/fpm/fpm_main.c | 12 +++++++----- ...fcgi-env-pif-apache-pp-sn-strip-encoded-plus.phpt | 2 +- .../fcgi-env-pif-apache-pp-sn-strip-encoded.phpt | 2 +- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 9d327f6bb951d..1a81c43a8a0d9 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,10 @@ PHP NEWS . Fixed bug GH-17718 (Calling static methods on an interface that has `__callStatic` is allowed). (timwolla) +- FPM: + . Fixed bug GH-17643 (FPM with httpd ProxyPass encoded PATH_INFO env). + (Jakub Zelenka) + - LDAP: . Fixed bug GH-17704 (ldap_search fails when $attributes contains a non-packed array with numerical keys). (nielsdos, 7u83) diff --git a/sapi/fpm/fpm/fpm_main.c b/sapi/fpm/fpm/fpm_main.c index a4b11627789ee..2a649b9a13501 100644 --- a/sapi/fpm/fpm/fpm_main.c +++ b/sapi/fpm/fpm/fpm_main.c @@ -1153,6 +1153,7 @@ static void init_request_info(void) } if (tflag) { + char *decoded_path_info = NULL; if (orig_path_info) { char old; @@ -1174,7 +1175,6 @@ static void init_request_info(void) * As we can extract PATH_INFO from PATH_TRANSLATED * it is probably also in SCRIPT_NAME and need to be removed */ - char *decoded_path_info = NULL; size_t decoded_path_info_len = 0; if (strchr(path_info, '%')) { decoded_path_info = estrdup(path_info); @@ -1197,11 +1197,13 @@ static void init_request_info(void) env_script_name[env_script_file_info_start] = 0; SG(request_info).request_uri = FCGI_PUTENV(request, "SCRIPT_NAME", env_script_name); } - if (decoded_path_info) { - efree(decoded_path_info); - } } - env_path_info = FCGI_PUTENV(request, "PATH_INFO", path_info); + if (decoded_path_info) { + env_path_info = FCGI_PUTENV(request, "PATH_INFO", decoded_path_info); + efree(decoded_path_info); + } else { + env_path_info = FCGI_PUTENV(request, "PATH_INFO", path_info); + } } if (!orig_script_filename || strcmp(orig_script_filename, pt) != 0) { diff --git a/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded-plus.phpt b/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded-plus.phpt index 4bef11ec668f0..06a974ce40693 100644 --- a/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded-plus.phpt +++ b/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded-plus.phpt @@ -39,7 +39,7 @@ $tester scriptFilename: "proxy:fcgi://" . $tester->getAddr() . $sourceFilePath . '/1%20+2', scriptName: $scriptName . '/1 +2' ) - ->expectBody([$scriptName, $scriptName . '/1 +2', $sourceFilePath, '/1%20+2', $scriptName . '/1%20+2']); + ->expectBody([$scriptName, $scriptName . '/1 +2', $sourceFilePath, '/1 +2', $scriptName . '/1 +2']); $tester->terminate(); $tester->close(); diff --git a/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded.phpt b/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded.phpt index 22114e1abde47..29834f3e13149 100644 --- a/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded.phpt +++ b/sapi/fpm/tests/fcgi-env-pif-apache-pp-sn-strip-encoded.phpt @@ -39,7 +39,7 @@ $tester scriptFilename: "proxy:fcgi://" . $tester->getAddr() . $sourceFilePath . '/1%202', scriptName: $scriptName . '/1 2' ) - ->expectBody([$scriptName, $scriptName . '/1 2', $sourceFilePath, '/1%202', $scriptName . '/1%202']); + ->expectBody([$scriptName, $scriptName . '/1 2', $sourceFilePath, '/1 2', $scriptName . '/1 2']); $tester->terminate(); $tester->close(); From 6ea1c7cb5b48d5eaa32331e1f50ac33460cad12f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Feb 2025 21:16:57 +0100 Subject: [PATCH 21/58] Add .gitignore section for additional test build files [ci skip] For now this is only for the bad_cmd.exe, but we may add more entries. Closes GH-17753. --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index 449963153f36b..b9e60cc6b5bde 100644 --- a/.gitignore +++ b/.gitignore @@ -291,6 +291,11 @@ tmp-php.ini /junit.out.xml /.ccache/ +# ------------------------------------------------------------------------------ +# Additional test build files +# ------------------------------------------------------------------------------ +/ext/standard/tests/helpers/bad_cmd.exe + # ------------------------------------------------------------------------------ # Special cases to invert previous ignore patterns # ------------------------------------------------------------------------------ From 4b5c29ef50113e6af06508f182a2790c6983744d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Feb 2025 15:13:33 +0100 Subject: [PATCH 22/58] Fix GH-17745: zlib extension incorrectly handles object arguments Because of the "H" modifier in ZPP, there are two bugs: 1) The stub is wrong and will cause a crash in debug mode. 2) Non-dynamic properties are not read correctly because they are not DEINDIRECTed. Closes GH-17750. --- NEWS | 4 ++++ ext/zlib/tests/gh17745.phpt | 20 ++++++++++++++++++++ ext/zlib/zlib.c | 6 ++++++ ext/zlib/zlib.stub.php | 4 ++-- ext/zlib/zlib_arginfo.h | 6 +++--- 5 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 ext/zlib/tests/gh17745.phpt diff --git a/NEWS b/NEWS index 1a81c43a8a0d9..45a8cbd96dee9 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,10 @@ PHP NEWS - Streams: . Fixed bug GH-17650 (realloc with size 0 in user_filters.c). (nielsdos) +- Zlib: + . Fixed bug GH-17745 (zlib extension incorrectly handles object arguments). + (nielsdos) + 13 Feb 2025, PHP 8.3.17 - Core: diff --git a/ext/zlib/tests/gh17745.phpt b/ext/zlib/tests/gh17745.phpt new file mode 100644 index 0000000000000..64331269dcdab --- /dev/null +++ b/ext/zlib/tests/gh17745.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-17745 (zlib extension incorrectly handles object arguments) +--EXTENSIONS-- +zlib +--FILE-- +level = 3; +var_dump(deflate_init(ZLIB_ENCODING_RAW, $obj)); + +class Options { + public int $level = 3; +} +var_dump(deflate_init(ZLIB_ENCODING_RAW, new Options)); +?> +--EXPECT-- +object(DeflateContext)#2 (0) { +} +object(DeflateContext)#3 (0) { +} diff --git a/ext/zlib/zlib.c b/ext/zlib/zlib.c index cfa8eefb47461..98b2fd6fe6ce4 100644 --- a/ext/zlib/zlib.c +++ b/ext/zlib/zlib.c @@ -790,6 +790,7 @@ static bool zlib_create_dictionary_string(HashTable *options, char **dict, size_ zval *option_buffer; if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("dictionary"))) != NULL) { + ZVAL_DEINDIRECT(option_buffer); ZVAL_DEREF(option_buffer); switch (Z_TYPE_P(option_buffer)) { case IS_STRING: { @@ -870,6 +871,7 @@ PHP_FUNCTION(inflate_init) } if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("window"))) != NULL) { + ZVAL_DEINDIRECT(option_buffer); window = zval_get_long(option_buffer); } if (window < 8 || window > 15) { @@ -1088,6 +1090,7 @@ PHP_FUNCTION(deflate_init) } if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("level"))) != NULL) { + ZVAL_DEINDIRECT(option_buffer); level = zval_get_long(option_buffer); } if (level < -1 || level > 9) { @@ -1096,6 +1099,7 @@ PHP_FUNCTION(deflate_init) } if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("memory"))) != NULL) { + ZVAL_DEINDIRECT(option_buffer); memory = zval_get_long(option_buffer); } if (memory < 1 || memory > 9) { @@ -1104,6 +1108,7 @@ PHP_FUNCTION(deflate_init) } if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("window"))) != NULL) { + ZVAL_DEINDIRECT(option_buffer); window = zval_get_long(option_buffer); } if (window < 8 || window > 15) { @@ -1112,6 +1117,7 @@ PHP_FUNCTION(deflate_init) } if (options && (option_buffer = zend_hash_str_find(options, ZEND_STRL("strategy"))) != NULL) { + ZVAL_DEINDIRECT(option_buffer); strategy = zval_get_long(option_buffer); } switch (strategy) { diff --git a/ext/zlib/zlib.stub.php b/ext/zlib/zlib.stub.php index 1083564f76505..09c44fec2e3a7 100644 --- a/ext/zlib/zlib.stub.php +++ b/ext/zlib/zlib.stub.php @@ -270,11 +270,11 @@ function gzread($stream, int $length): string|false {} */ function gzgets($stream, ?int $length = null): string|false {} -function deflate_init(int $encoding, array $options = []): DeflateContext|false {} +function deflate_init(int $encoding, array|object $options = []): DeflateContext|false {} function deflate_add(DeflateContext $context, string $data, int $flush_mode = ZLIB_SYNC_FLUSH): string|false {} -function inflate_init(int $encoding, array $options = []): InflateContext|false {} +function inflate_init(int $encoding, array|object $options = []): InflateContext|false {} function inflate_add(InflateContext $context, string $data, int $flush_mode = ZLIB_SYNC_FLUSH): string|false {} diff --git a/ext/zlib/zlib_arginfo.h b/ext/zlib/zlib_arginfo.h index 743662fd036bd..1f0e082cae85f 100644 --- a/ext/zlib/zlib_arginfo.h +++ b/ext/zlib/zlib_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 3660ad3239f93c84b6909c36ddfcc92dd0773c70 */ + * Stub hash: a86ccc292b5312e740a9d798bfcaf014513d5cce */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_ob_gzhandler, 0, 2, MAY_BE_STRING|MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, data, IS_STRING, 0) @@ -106,7 +106,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_deflate_init, 0, 1, DeflateContext, MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, encoding, IS_LONG, 0) - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_ARRAY, 0, "[]") + ZEND_ARG_TYPE_MASK(0, options, MAY_BE_ARRAY|MAY_BE_OBJECT, "[]") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_deflate_add, 0, 2, MAY_BE_STRING|MAY_BE_FALSE) @@ -117,7 +117,7 @@ ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_OBJ_TYPE_MASK_EX(arginfo_inflate_init, 0, 1, InflateContext, MAY_BE_FALSE) ZEND_ARG_TYPE_INFO(0, encoding, IS_LONG, 0) - ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, options, IS_ARRAY, 0, "[]") + ZEND_ARG_TYPE_MASK(0, options, MAY_BE_ARRAY|MAY_BE_OBJECT, "[]") ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_inflate_add, 0, 2, MAY_BE_STRING|MAY_BE_FALSE) From a54af45a41892b9d6165124a20f9571ca8700b50 Mon Sep 17 00:00:00 2001 From: ndossche Date: Thu, 13 Feb 2025 16:53:43 +0100 Subject: [PATCH 23/58] Fix memory leak when encoding check fails zlib_create_dictionary_string() allocates memory, so we can leak memory if there's an early exit before the assignment to the return value. Solve this by moving all validation upwards. Closes GH-17788. --- NEWS | 1 + .../leak_invalid_encoding_with_dict.phpt | 20 +++++++++++++++++++ ext/zlib/zlib.c | 16 +++++++-------- 3 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 ext/zlib/tests/leak_invalid_encoding_with_dict.phpt diff --git a/NEWS b/NEWS index 45a8cbd96dee9..40eb7b37c7055 100644 --- a/NEWS +++ b/NEWS @@ -41,6 +41,7 @@ PHP NEWS - Zlib: . Fixed bug GH-17745 (zlib extension incorrectly handles object arguments). (nielsdos) + . Fix memory leak when encoding check fails. (nielsdos) 13 Feb 2025, PHP 8.3.17 diff --git a/ext/zlib/tests/leak_invalid_encoding_with_dict.phpt b/ext/zlib/tests/leak_invalid_encoding_with_dict.phpt new file mode 100644 index 0000000000000..da2a11849c0c2 --- /dev/null +++ b/ext/zlib/tests/leak_invalid_encoding_with_dict.phpt @@ -0,0 +1,20 @@ +--TEST-- +Memory leak when passing a dictionary with invalid encoding +--EXTENSIONS-- +zlib +--FILE-- + "dict"]); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +try { + deflate_init(123456, ["dictionary" => "dict"]); +} catch (ValueError $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECT-- +Encoding mode must be ZLIB_ENCODING_RAW, ZLIB_ENCODING_GZIP or ZLIB_ENCODING_DEFLATE +deflate_init(): Argument #1 ($encoding) must be one of ZLIB_ENCODING_RAW, ZLIB_ENCODING_GZIP, or ZLIB_ENCODING_DEFLATE diff --git a/ext/zlib/zlib.c b/ext/zlib/zlib.c index 98b2fd6fe6ce4..a97d32c6d43eb 100644 --- a/ext/zlib/zlib.c +++ b/ext/zlib/zlib.c @@ -879,10 +879,6 @@ PHP_FUNCTION(inflate_init) RETURN_THROWS(); } - if (!zlib_create_dictionary_string(options, &dict, &dictlen)) { - RETURN_THROWS(); - } - switch (encoding) { case PHP_ZLIB_ENCODING_RAW: case PHP_ZLIB_ENCODING_GZIP: @@ -893,6 +889,10 @@ PHP_FUNCTION(inflate_init) RETURN_THROWS(); } + if (!zlib_create_dictionary_string(options, &dict, &dictlen)) { + RETURN_THROWS(); + } + object_init_ex(return_value, inflate_context_ce); ctx = Z_INFLATE_CONTEXT_P(return_value); @@ -1132,10 +1132,6 @@ PHP_FUNCTION(deflate_init) RETURN_THROWS(); } - if (!zlib_create_dictionary_string(options, &dict, &dictlen)) { - RETURN_THROWS(); - } - switch (encoding) { case PHP_ZLIB_ENCODING_RAW: case PHP_ZLIB_ENCODING_GZIP: @@ -1146,6 +1142,10 @@ PHP_FUNCTION(deflate_init) RETURN_THROWS(); } + if (!zlib_create_dictionary_string(options, &dict, &dictlen)) { + RETURN_THROWS(); + } + object_init_ex(return_value, deflate_context_ce); ctx = Z_DEFLATE_CONTEXT_P(return_value); From 4d7d01d18ea10257bcb2cd7e23ad5d39a7aecd6f Mon Sep 17 00:00:00 2001 From: David Carlier Date: Wed, 12 Feb 2025 23:17:33 +0000 Subject: [PATCH 24/58] Fix GH-17772: imagepalettetotruecolor segfault on invalid truecolor pixel. close GH-17777 --- NEWS | 4 ++++ ext/gd/libgd/gd.c | 6 +++++- ext/gd/tests/gh17772.phpt | 28 ++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 ext/gd/tests/gh17772.phpt diff --git a/NEWS b/NEWS index 40eb7b37c7055..84999491ab0ac 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,10 @@ PHP NEWS . Fixed bug GH-17643 (FPM with httpd ProxyPass encoded PATH_INFO env). (Jakub Zelenka) +- GD: + . Fixed bug GH-17772 (imagepalettetotruecolor crash with memory_limit=2M). + (David Carlier) + - LDAP: . Fixed bug GH-17704 (ldap_search fails when $attributes contains a non-packed array with numerical keys). (nielsdos, 7u83) diff --git a/ext/gd/libgd/gd.c b/ext/gd/libgd/gd.c index 7265758696ad3..0bd6e4b587e9f 100644 --- a/ext/gd/libgd/gd.c +++ b/ext/gd/libgd/gd.c @@ -3108,7 +3108,11 @@ int gdImagePaletteToTrueColor(gdImagePtr src) const unsigned int sy = gdImageSY(src); const unsigned int sx = gdImageSX(src); - src->tpixels = (int **) gdMalloc(sizeof(int *) * sy); + // Note: do not revert back to gdMalloc() below ; reason here, + // due to a bug with a certain memory_limit INI value treshold, + // imagepalettetotruecolor crashes with even unrelated ZendMM allocations. + // See GH-17772 for an use case. + src->tpixels = (int **) gdCalloc(sizeof(int *), sy); if (src->tpixels == NULL) { return 0; } diff --git a/ext/gd/tests/gh17772.phpt b/ext/gd/tests/gh17772.phpt new file mode 100644 index 0000000000000..6252a13341f41 --- /dev/null +++ b/ext/gd/tests/gh17772.phpt @@ -0,0 +1,28 @@ +--TEST-- +GH-17772 (imagepalettetotruecolor segfault on image deallocation) +--EXTENSIONS-- +gd +--INI-- +memory_limit=2M +--CREDITS-- +YuanchengJiang +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d From 7f6c05116e83e75353f27f5333cc860c3a6f64f7 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Mon, 10 Feb 2025 17:44:33 +0100 Subject: [PATCH 25/58] [skip ci] Fix phpize for Windows 11 (24H2) It seems like n === undefined must have worked on older versions of jscript, but currently it just causes the insertion to silently fail. This sets n to an empty string, allowing phpize to include the local config.w32 files. --- NEWS | 7 +++++-- win32/build/phpize.js.in | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index aa23510db1753..7324aaf5bef54 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,10 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| +?? ??? ????, PHP 8.1.32 + +- Windows: + . Fixed phpize for Windows 11 (24H2). (bwoebi) + 21 Nov 2024, PHP 8.1.31 - CLI: @@ -28,8 +33,6 @@ PHP NEWS . Fixed bug GHSA-r977-prxv-hc43 (Single byte overread with convert.quoted-printable-decode filter). (CVE-2024-11233) (nielsdos) - - 26 Sep 2024, PHP 8.1.30 - CGI: diff --git a/win32/build/phpize.js.in b/win32/build/phpize.js.in index 49871481e8907..c5f57737c97d0 100644 --- a/win32/build/phpize.js.in +++ b/win32/build/phpize.js.in @@ -91,6 +91,7 @@ function find_config_w32(dirname) deps = get_module_dep(contents); + n = ""; item = new Module_Item(n, c, dir_line, deps, contents); MODULES.Add(n, item); } From 678ecff9808ab9fd340e9110935b02de4ab73543 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 13 Feb 2025 19:49:13 +0100 Subject: [PATCH 26/58] Fix memory leak on overflow in _php_stream_scandir() On overflow, only the array is freed, but not the strings. Closes GH-17789. --- NEWS | 1 + main/streams/streams.c | 24 +++++++++++++----------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index bbc96dd924a00..9493e4f6af7ee 100644 --- a/NEWS +++ b/NEWS @@ -41,6 +41,7 @@ PHP NEWS - Streams: . Fixed bug GH-17650 (realloc with size 0 in user_filters.c). (nielsdos) + . Fix memory leak on overflow in _php_stream_scandir(). (nielsdos) - Windows: . Fixed phpize for Windows 11 (24H2). (bwoebi) diff --git a/main/streams/streams.c b/main/streams/streams.c index 934ed14211508..7a5dc2a58aaf1 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -2469,25 +2469,19 @@ PHPAPI int _php_stream_scandir(const char *dirname, zend_string **namelist[], in vector_size = 10; } else { if(vector_size*2 < vector_size) { - /* overflow */ - php_stream_closedir(stream); - efree(vector); - return -1; + goto overflow; } vector_size *= 2; } - vector = (zend_string **) safe_erealloc(vector, vector_size, sizeof(char *), 0); + vector = (zend_string **) safe_erealloc(vector, vector_size, sizeof(zend_string *), 0); } vector[nfiles] = zend_string_init(sdp.d_name, strlen(sdp.d_name), 0); - nfiles++; - if(vector_size < 10 || nfiles == 0) { - /* overflow */ - php_stream_closedir(stream); - efree(vector); - return -1; + if(vector_size < 10 || nfiles + 1 == 0) { + goto overflow; } + nfiles++; } php_stream_closedir(stream); @@ -2497,5 +2491,13 @@ PHPAPI int _php_stream_scandir(const char *dirname, zend_string **namelist[], in qsort(*namelist, nfiles, sizeof(zend_string *), (int(*)(const void *, const void *))compare); } return nfiles; + +overflow: + php_stream_closedir(stream); + for (unsigned int i = 0; i < nfiles; i++) { + zend_string_efree(vector[i]); + } + efree(vector); + return -1; } /* }}} */ From 5aaf7b49374e6c55bb5cd8b8544068107706a7a2 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 12 Feb 2025 22:37:29 +0100 Subject: [PATCH 27/58] Fix zlib support for large files gzread() and gzwrite() have effectively a 4GiB limit at the moment because the APIs of the zlib library use unsigned ints. For example, this means that the count argument of gzread() and gzwrite() & co effectively are modulo 2**32. Fix this by adding a loop to handle all bytes. As for automated testing, I didn't find an easy way to write a phpt for this that wouldn't use a lot of memory or requires a large file. For instance, the gzread() test that I manually ran requires a 4MiB input file (and I can't shrink it because zlib has a max window size). Here are the testing instructions, run on 64-bit: To test for gzwrite(): ```php $f = gzopen("out.txt.gz", "w"); gzwrite($f, str_repeat('a', 4*1024*1024*1024+64)); // 4GiB + 64 bytes ``` Then use `zcat out.txt.gz|wc -c` to check that all bytes were written (should be 4294967360). To test for gzread(): Create a file containing all a's for example that is 4GiB + 64 bytes. Then compress it into out.txt.gz using the gzip command. Then run: ```php $f = gzopen("out.txt.gz", "r"); $str = gzread($f, 4*1024*1024*1024+64); var_dump(strlen($str)); // 4294967360 var_dump(substr($str, -3)); // string (3) "aaa" ``` Closes GH-17775. --- NEWS | 1 + ext/zlib/zlib_fopen_wrapper.c | 49 ++++++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/NEWS b/NEWS index 9493e4f6af7ee..876ba5829069b 100644 --- a/NEWS +++ b/NEWS @@ -50,6 +50,7 @@ PHP NEWS . Fixed bug GH-17745 (zlib extension incorrectly handles object arguments). (nielsdos) . Fix memory leak when encoding check fails. (nielsdos) + . Fix zlib support for large files. (nielsdos) 13 Feb 2025, PHP 8.3.17 diff --git a/ext/zlib/zlib_fopen_wrapper.c b/ext/zlib/zlib_fopen_wrapper.c index b414b33a8724e..31b5212a720ac 100644 --- a/ext/zlib/zlib_fopen_wrapper.c +++ b/ext/zlib/zlib_fopen_wrapper.c @@ -33,24 +33,55 @@ struct php_gz_stream_data_t { static ssize_t php_gziop_read(php_stream *stream, char *buf, size_t count) { struct php_gz_stream_data_t *self = (struct php_gz_stream_data_t *) stream->abstract; - int read; + ssize_t total_read = 0; + + /* Despite the count argument of gzread() being "unsigned int", + * the return value is "int". Error returns are values < 0, otherwise the count is returned. + * To properly distinguish error values from success value, we therefore need to cap at INT_MAX. + */ + do { + unsigned int chunk_size = MIN(count, INT_MAX); + int read = gzread(self->gz_file, buf, chunk_size); + count -= chunk_size; + + if (gzeof(self->gz_file)) { + stream->eof = 1; + } - /* XXX this needs to be looped for the case count > UINT_MAX */ - read = gzread(self->gz_file, buf, count); + if (UNEXPECTED(read < 0)) { + return read; + } - if (gzeof(self->gz_file)) { - stream->eof = 1; - } + total_read += read; + buf += read; + } while (count > 0 && !stream->eof); - return read; + return total_read; } static ssize_t php_gziop_write(php_stream *stream, const char *buf, size_t count) { struct php_gz_stream_data_t *self = (struct php_gz_stream_data_t *) stream->abstract; + ssize_t total_written = 0; + + /* Despite the count argument of gzread() being "unsigned int", + * the return value is "int". Error returns are values < 0, otherwise the count is returned. + * To properly distinguish error values from success value, we therefore need to cap at INT_MAX. + */ + do { + unsigned int chunk_size = MIN(count, INT_MAX); + int written = gzwrite(self->gz_file, buf, chunk_size); + count -= chunk_size; + + if (UNEXPECTED(written < 0)) { + return written; + } + + total_written += written; + buf += written; + } while (count > 0); - /* XXX this needs to be looped for the case count > UINT_MAX */ - return gzwrite(self->gz_file, (char *) buf, count); + return total_written; } static int php_gziop_seek(php_stream *stream, zend_off_t offset, int whence, zend_off_t *newoffs) From f4aadb5537f87afb4ffd5133ff92532f2c1d8e15 Mon Sep 17 00:00:00 2001 From: Shivam Mathur Date: Thu, 30 Jan 2025 11:46:57 +0000 Subject: [PATCH 28/58] ci: add workflow to trigger windows builds --- .github/workflows/windows-builds.yml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/windows-builds.yml diff --git a/.github/workflows/windows-builds.yml b/.github/workflows/windows-builds.yml new file mode 100644 index 0000000000000..6bb4cd897164d --- /dev/null +++ b/.github/workflows/windows-builds.yml @@ -0,0 +1,23 @@ +name: Windows builds +run-name: Windows builds for ${{ inputs.tag || github.ref_name }} +on: + push: + tags: + - 'php-*' + workflow_dispatch: + inputs: + tag: + description: 'Tag version' + required: true + +jobs: + publish: + runs-on: ubuntu-latest + name: Build + steps: + - name: Build + env: + GITHUB_TOKEN: ${{ secrets.WINDOWS_BUILDS_TOKEN }} + run: | + TAG="${{ inputs.tag || github.ref_name }}" + gh workflow run php.yml -R php/php-windows-builder -f php-version="${TAG#php-}" From 0f63bee3e99443b469ff53f423d3ea3303fb7fd3 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 14 Feb 2025 07:06:42 +0000 Subject: [PATCH 29/58] Fix GH-17797: zend_test_compile_string crash on invalid script path. When looking for the last slash of the script path, it leads to underflow being promoted to SIZE_MAX being way beyond MAXPATHLEN. close GH-17801 --- NEWS | 2 ++ ext/zend_test/tests/gh17797.phpt | 31 +++++++++++++++++++++++++++++++ main/fopen_wrappers.c | 8 +++++++- 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 ext/zend_test/tests/gh17797.phpt diff --git a/NEWS b/NEWS index 876ba5829069b..42a0ec193d9fd 100644 --- a/NEWS +++ b/NEWS @@ -13,6 +13,8 @@ PHP NEWS . Fix fallback paths in fast_long_{add,sub}_function. (nielsdos) . Fixed bug GH-17718 (Calling static methods on an interface that has `__callStatic` is allowed). (timwolla) + . Fixed bug GH-17797 (zend_test_compile_string crash on invalid + script path). (David Carlier) - FPM: . Fixed bug GH-17643 (FPM with httpd ProxyPass encoded PATH_INFO env). diff --git a/ext/zend_test/tests/gh17797.phpt b/ext/zend_test/tests/gh17797.phpt new file mode 100644 index 0000000000000..9ae1bedb67425 --- /dev/null +++ b/ext/zend_test/tests/gh17797.phpt @@ -0,0 +1,31 @@ +--TEST-- +GH-17797 (zend_test_compile_string crash on invalid script path) +--EXTENSIONS-- +zend_test +--CREDITS-- +YuanchengJiang +--FILE-- +'; +try {zend_test_compile_string($source,$source,$c);} catch (Exception $e) { echo($e); } +--EXPECTF-- + +Warning: Undefined variable $c in %s on line %d + +Deprecated: zend_test_compile_string(): Passing null to parameter #3 ($position) of type int is deprecated in %s on line %d + +Warning: require(sumfile.php): Failed to open stream: No such file or directory in on line %d + +Fatal error: Uncaught Error: Failed opening required 'sumfile.php' (include_path='.%s') in :%d +Stack trace: +#0 %s(%d): zend_test_compile_string(' on line %d diff --git a/main/fopen_wrappers.c b/main/fopen_wrappers.c index 62f6b4de40433..3a1f452a644cd 100644 --- a/main/fopen_wrappers.c +++ b/main/fopen_wrappers.c @@ -603,7 +603,13 @@ PHPAPI zend_string *php_resolve_path(const char *filename, size_t filename_lengt const char *exec_fname = ZSTR_VAL(exec_filename); size_t exec_fname_length = ZSTR_LEN(exec_filename); - while ((--exec_fname_length < SIZE_MAX) && !IS_SLASH(exec_fname[exec_fname_length])); + while (exec_fname_length > 0) { + --exec_fname_length; + if (IS_SLASH(exec_fname[exec_fname_length])) { + break; + } + } + if (exec_fname_length > 0 && filename_length < (MAXPATHLEN - 2) && exec_fname_length + 1 + filename_length + 1 < MAXPATHLEN) { From e735d2bc3b41c40cea45fdaefabd3cd48f100e0a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 15 Feb 2025 12:38:55 +0100 Subject: [PATCH 30/58] Fix GH-17808: PharFileInfo refcount bug PharFileInfo just takes a pointer from the manifest without refcounting anything. If the entry is then removed from the manifest while the PharFileInfo object still exists, we get a UAF. We fix this by using the fp_refcount field. This is technically a behaviour change as the unlinking is now blocked, and potentially file modifications can be blocked as well. The alternative would be to have a field that indicates whether deletion is blocked, but similar corruption bugs may occur as well with file overwrites, so we increment fp_refcount instead. This also fixes an issue where a destructor called multiple times resulted in a UAF as well, by moving the NULL'ing of the entry field out of the if. Closes GH-17811. --- NEWS | 3 +++ ext/phar/phar_object.c | 15 +++++++++++++-- ext/phar/tests/gh17808.phpt | 21 +++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 ext/phar/tests/gh17808.phpt diff --git a/NEWS b/NEWS index 42a0ec193d9fd..c71825e7304b9 100644 --- a/NEWS +++ b/NEWS @@ -37,6 +37,9 @@ PHP NEWS JIT crash). (nielsdos) . Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry) +- Phar: + . Fixed bug GH-17808: PharFileInfo refcount bug. (nielsdos) + - PHPDBG: . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) . Fix memory leak in phpdbg calling registered function. (nielsdos) diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 39526ce4b235b..bd724b1036949 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -4483,6 +4483,9 @@ PHP_METHOD(PharFileInfo, __construct) efree(entry); entry_obj->entry = entry_info; + if (!entry_info->is_persistent && !entry_info->is_temp_dir) { + ++entry_info->fp_refcount; + } ZVAL_STRINGL(&arg1, fname, fname_len); @@ -4512,15 +4515,23 @@ PHP_METHOD(PharFileInfo, __destruct) RETURN_THROWS(); } - if (entry_obj->entry && entry_obj->entry->is_temp_dir) { + if (!entry_obj->entry) { + return; + } + + if (entry_obj->entry->is_temp_dir) { if (entry_obj->entry->filename) { efree(entry_obj->entry->filename); entry_obj->entry->filename = NULL; } efree(entry_obj->entry); - entry_obj->entry = NULL; + } else if (!entry_obj->entry->is_persistent) { + --entry_obj->entry->fp_refcount; + /* It is necessarily still in the manifest, which will ultimately free this. */ } + + entry_obj->entry = NULL; } /* }}} */ diff --git a/ext/phar/tests/gh17808.phpt b/ext/phar/tests/gh17808.phpt new file mode 100644 index 0000000000000..28cb9a2e22792 --- /dev/null +++ b/ext/phar/tests/gh17808.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-17808 (PharFileInfo refcount bug) +--EXTENSIONS-- +phar +--FILE-- +getContent())); +unlink("$file"); +var_dump($file->getATime()); +?> +--EXPECTF-- +string(%d) "phar://%spackage.xml" +int(6747) + +Warning: unlink(): phar error: "package.xml" in phar %s, has open file pointers, cannot unlink in %s on line %d +int(33188) From 3e879f3dd5ca4fe605be11b7a22fbec7bcc0701d Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 16 Feb 2025 00:16:57 +0100 Subject: [PATCH 31/58] [ci skip] Fix GH-17808 dependencies --- ext/phar/tests/gh17808.phpt | 1 + 1 file changed, 1 insertion(+) diff --git a/ext/phar/tests/gh17808.phpt b/ext/phar/tests/gh17808.phpt index 28cb9a2e22792..03e54ff264bfa 100644 --- a/ext/phar/tests/gh17808.phpt +++ b/ext/phar/tests/gh17808.phpt @@ -2,6 +2,7 @@ GH-17808 (PharFileInfo refcount bug) --EXTENSIONS-- phar +zlib --FILE-- Date: Fri, 14 Feb 2025 15:10:31 +0100 Subject: [PATCH 32/58] Prevent using system DLLs when running the tests The search order for DLLs on Windows is (simplified): * the application folder * the system folder * all folders in the `PATH` (The full details are documented on Microsoft Learn[1].) As is, we're adding `deps\bin` to the `PATH` when running the tests, but any DLLs in the system folder take precedence, so these would be used instead of our intended dependencies. To mitigate that, we copy over all DLLs from `deps\bin` to our application folder (i.e. where php.exe, php-cgi.exe and phpdbg.exe are placed). Since we're doing this, there is no more need to attempt to remove the OpenSSL DLLs in the system folder (what seems to be a bad idea anyway). [1] Closes GH-17805. --- .github/scripts/windows/build_task.bat | 5 ----- .github/scripts/windows/test_task.bat | 2 ++ 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/scripts/windows/build_task.bat b/.github/scripts/windows/build_task.bat index e8d84b8c0bfd6..071c0c28f5ac5 100644 --- a/.github/scripts/windows/build_task.bat +++ b/.github/scripts/windows/build_task.bat @@ -5,11 +5,6 @@ if /i "%GITHUB_ACTIONS%" neq "True" ( exit /b 3 ) -del /f /q C:\Windows\System32\libcrypto-1_1-x64.dll >NUL 2>NUL -if %errorlevel% neq 0 exit /b 3 -del /f /q C:\Windows\System32\libssl-1_1-x64.dll >NUL 2>NUL -if %errorlevel% neq 0 exit /b 3 - call %~dp0find-target-branch.bat set STABILITY=staging set DEPS_DIR=%PHP_BUILD_CACHE_BASE_DIR%\deps-%BRANCH%-%PHP_SDK_VS%-%PHP_SDK_ARCH% diff --git a/.github/scripts/windows/test_task.bat b/.github/scripts/windows/test_task.bat index 0bfdaf664125b..5762aa32414e4 100644 --- a/.github/scripts/windows/test_task.bat +++ b/.github/scripts/windows/test_task.bat @@ -137,6 +137,8 @@ for %%i in (ldap oci8_12c pdo_oci) do ( set TEST_PHPDBG_EXECUTABLE=%PHP_BUILD_DIR%\phpdbg.exe +copy /-y %DEPS_DIR%\bin\*.dll %PHP_BUILD_DIR%\* + mkdir c:\tests_tmp nmake test TESTS="%OPCACHE_OPTS% -g FAIL,BORK,LEAK,XLEAK --no-progress -q --offline --show-diff --show-slow 1000 --set-timeout 120 --temp-source c:\tests_tmp --temp-target c:\tests_tmp --bless %PARALLEL%" From 765cebf73a70f4fcd406b0bd44255358db377dc6 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 11 Feb 2025 20:07:26 +0100 Subject: [PATCH 33/58] Adapt tests to Windows 11 Apparently, one of the more recent patch releases of Windows 10 (confirmed for Windows 10.0.26100, but may affect older versions, too) changed treatment of filenames with trailing slashes to be recognized explicitly as directories, and no longer as invalid file or directory. We adapt the affected test cases. Closes GH-17804. --- ext/standard/tests/file/rename_variation-win32.phpt | 2 +- ext/standard/tests/file/unlink_variation8-win32.phpt | 4 ++-- ext/standard/tests/file/unlink_variation9-win32.phpt | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/standard/tests/file/rename_variation-win32.phpt b/ext/standard/tests/file/rename_variation-win32.phpt index dc7586d5042b7..a348438bed556 100644 --- a/ext/standard/tests/file/rename_variation-win32.phpt +++ b/ext/standard/tests/file/rename_variation-win32.phpt @@ -61,7 +61,7 @@ bool(false) bool(true) -- Iteration 2 -- -Warning: rename(%s/rename_variation-win32/rename_variation.tmp/,%s/rename_variation2.tmp): The filename, directory name, or volume label syntax is incorrect (code: 123) in %s on line %d +Warning: rename(%s/rename_variation-win32/rename_variation.tmp/,%s/rename_variation2.tmp): %rThe filename, directory name, or volume label syntax is incorrect|The directory name is invalid%r (code: %r123|267%r) in %s on line %d bool(false) bool(false) bool(false) diff --git a/ext/standard/tests/file/unlink_variation8-win32.phpt b/ext/standard/tests/file/unlink_variation8-win32.phpt index 12ace11fc196c..016beddce1c13 100644 --- a/ext/standard/tests/file/unlink_variation8-win32.phpt +++ b/ext/standard/tests/file/unlink_variation8-win32.phpt @@ -95,10 +95,10 @@ file removed Warning: unlink(%s/BADDIR/file.tmp): No such file or directory in %s on line %d -- removing unlinkVar8.tmp/file.tmp/ -- -Warning: unlink(unlinkVar8.tmp/file.tmp/): No such file or directory in %s on line %d +Warning: unlink(unlinkVar8.tmp/file.tmp/): %rNo such file or directory|Not a directory%r in %s on line %d -- removing %s/unlinkVar8.tmp/file.tmp/ -- -Warning: unlink(%s/unlinkVar8.tmp/file.tmp/): No such file or directory in %s on line %d +Warning: unlink(%s/unlinkVar8.tmp/file.tmp/): %rNo such file or directory|Not a directory%r in %s on line %d -- removing unlinkVar8.tmp//file.tmp -- file removed -- removing %s//unlinkVar8.tmp//file.tmp -- diff --git a/ext/standard/tests/file/unlink_variation9-win32.phpt b/ext/standard/tests/file/unlink_variation9-win32.phpt index a69c27088e94a..68e11be927637 100644 --- a/ext/standard/tests/file/unlink_variation9-win32.phpt +++ b/ext/standard/tests/file/unlink_variation9-win32.phpt @@ -97,10 +97,10 @@ file removed Warning: unlink(%s\BADDIR\file.tmp): No such file or directory in %s on line %d -- removing unlinkVar9.tmp\file.tmp\ -- -Warning: unlink(unlinkVar9.tmp\file.tmp\): No such file or directory in %s on line %d +Warning: unlink(unlinkVar9.tmp\file.tmp\): %rNo such file or directory|Not a directory%r in %s on line %d -- removing %s\unlinkVar9.tmp\file.tmp\ -- -Warning: unlink(%s\unlinkVar9.tmp\file.tmp\): No such file or directory in %s on line %d +Warning: unlink(%s\unlinkVar9.tmp\file.tmp\): %rNo such file or directory|Not a directory%r in %s on line %d -- removing unlinkVar9.tmp\\file.tmp -- file removed -- removing %s\\unlinkVar9.tmp\\file.tmp -- From 52c91f0fb707efde04fb32667178e4246c7ffe23 Mon Sep 17 00:00:00 2001 From: David Zhong <91637806+davnotdev@users.noreply.github.com> Date: Sun, 16 Feb 2025 23:27:11 -0800 Subject: [PATCH 34/58] Fix FFI Parsing of Pointer Declaration Lists (#17794) * Fix ffi parsing of pointer declaration lists * Fix ffi pointer declaration lists grammar --- ext/ffi/ffi.g | 4 ++- ext/ffi/ffi_parser.c | 4 ++- ext/ffi/tests/ptr_declaration_list.phpt | 33 +++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 ext/ffi/tests/ptr_declaration_list.phpt diff --git a/ext/ffi/ffi.g b/ext/ffi/ffi.g index d70075267e54f..3648372428900 100644 --- a/ext/ffi/ffi.g +++ b/ext/ffi/ffi.g @@ -264,12 +264,14 @@ struct_contents(zend_ffi_dcl *dcl): struct_declaration(zend_ffi_dcl *struct_dcl): {zend_ffi_dcl common_field_dcl = ZEND_FFI_ATTR_INIT;} + {zend_ffi_dcl base_field_dcl = ZEND_FFI_ATTR_INIT;} specifier_qualifier_list(&common_field_dcl) + {base_field_dcl = common_field_dcl;} ( /* empty */ {zend_ffi_add_anonymous_field(struct_dcl, &common_field_dcl);} | struct_declarator(struct_dcl, &common_field_dcl) ( "," - {zend_ffi_dcl field_dcl = common_field_dcl;} + {zend_ffi_dcl field_dcl = base_field_dcl;} attributes(&field_dcl)? struct_declarator(struct_dcl, &field_dcl) )* diff --git a/ext/ffi/ffi_parser.c b/ext/ffi/ffi_parser.c index 2589ae81e0259..26d623a40290e 100644 --- a/ext/ffi/ffi_parser.c +++ b/ext/ffi/ffi_parser.c @@ -2472,14 +2472,16 @@ static int parse_struct_contents(int sym, zend_ffi_dcl *dcl) { static int parse_struct_declaration(int sym, zend_ffi_dcl *struct_dcl) { zend_ffi_dcl common_field_dcl = ZEND_FFI_ATTR_INIT; + zend_ffi_dcl base_field_dcl = ZEND_FFI_ATTR_INIT; sym = parse_specifier_qualifier_list(sym, &common_field_dcl); + base_field_dcl = common_field_dcl; if (sym == YY__SEMICOLON || sym == YY__RBRACE) { zend_ffi_add_anonymous_field(struct_dcl, &common_field_dcl); } else if (sym == YY__STAR || sym == YY_ID || sym == YY__LPAREN || sym == YY__COLON) { sym = parse_struct_declarator(sym, struct_dcl, &common_field_dcl); while (sym == YY__COMMA) { sym = get_sym(); - zend_ffi_dcl field_dcl = common_field_dcl; + zend_ffi_dcl field_dcl = base_field_dcl; if (YY_IN_SET(sym, (YY___ATTRIBUTE,YY___ATTRIBUTE__,YY___DECLSPEC,YY___CDECL,YY___STDCALL,YY___FASTCALL,YY___THISCALL,YY___VECTORCALL), "\000\000\000\000\000\000\360\017\000\000\000\000\000")) { sym = parse_attributes(sym, &field_dcl); } diff --git a/ext/ffi/tests/ptr_declaration_list.phpt b/ext/ffi/tests/ptr_declaration_list.phpt new file mode 100644 index 0000000000000..cb8ecafd935ba --- /dev/null +++ b/ext/ffi/tests/ptr_declaration_list.phpt @@ -0,0 +1,33 @@ +--TEST-- +Declaration Lists with Pointers +--EXTENSIONS-- +ffi +--SKIPIF-- +--FILE-- +new('struct MyStruct'); +$one = $ffi->new("uint8_t"); +$oneptr = $ffi->new("uint8_t*"); +$oneptrptr = $ffi->new("uint8_t**"); +$one->cdata = 1; +$oneptr = FFI::addr($one); +$oneptrptr = FFI::addr($oneptr); + +$test_struct->a = $oneptrptr; +$test_struct->b = $oneptr; +$test_struct->c = $one; + +var_dump($test_struct->a[0][0]); +var_dump($test_struct->b[0]); +var_dump($test_struct->c); +?> +--EXPECT-- +int(1) +int(1) +int(1) From 86f5a31e5b0bcb19f074151a9091d42e2a41a706 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 17 Feb 2025 21:58:11 +0100 Subject: [PATCH 35/58] NEWS for GH-17794 [ci skip] --- NEWS | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS b/NEWS index c71825e7304b9..7c20204a62e22 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,9 @@ PHP NEWS . Fixed bug GH-17797 (zend_test_compile_string crash on invalid script path). (David Carlier) +- FFI: + . Fix FFI Parsing of Pointer Declaration Lists. (davnotdev) + - FPM: . Fixed bug GH-17643 (FPM with httpd ProxyPass encoded PATH_INFO env). (Jakub Zelenka) From 260e0e9bd3712e37da36c81afe8c0c22f86aae0f Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 18 Feb 2025 13:45:31 +0100 Subject: [PATCH 36/58] Fix GH-17837: ::getColumnMeta() on unexecuted statement segfaults We cannot properly get the column meta data of a statement which has been prepared, but has not yet been executed. As such we bail out early, reporting failure. Closes GH-17850. --- NEWS | 4 ++++ ext/pdo_sqlite/sqlite_statement.c | 2 +- ext/pdo_sqlite/tests/gh17837.phpt | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 ext/pdo_sqlite/tests/gh17837.phpt diff --git a/NEWS b/NEWS index 7c20204a62e22..af80e9a4bb61c 100644 --- a/NEWS +++ b/NEWS @@ -40,6 +40,10 @@ PHP NEWS JIT crash). (nielsdos) . Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry) +- PDO_SQLite: + . Fixed GH-17837 ()::getColumnMeta() on unexecuted statement segfaults). + (cmb) + - Phar: . Fixed bug GH-17808: PharFileInfo refcount bug. (nielsdos) diff --git a/ext/pdo_sqlite/sqlite_statement.c b/ext/pdo_sqlite/sqlite_statement.c index c6b907f6fc22f..16aac6095af1a 100644 --- a/ext/pdo_sqlite/sqlite_statement.c +++ b/ext/pdo_sqlite/sqlite_statement.c @@ -305,7 +305,7 @@ static int pdo_sqlite_stmt_col_meta(pdo_stmt_t *stmt, zend_long colno, zval *ret const char *str; zval flags; - if (!S->stmt) { + if (!S->stmt || !stmt->executed) { return FAILURE; } if(colno >= sqlite3_column_count(S->stmt)) { diff --git a/ext/pdo_sqlite/tests/gh17837.phpt b/ext/pdo_sqlite/tests/gh17837.phpt new file mode 100644 index 0000000000000..c8e1f6dab2829 --- /dev/null +++ b/ext/pdo_sqlite/tests/gh17837.phpt @@ -0,0 +1,14 @@ +--TEST-- +GH-17837 (::getColumnMeta() on unexecuted statement segfaults) +--EXTENSIONS-- +pdo_sqlite +--CREDITS-- +YuanchengJiang +--FILE-- +prepare('select :a, :b, ?'); +var_dump($stmt->getColumnMeta(0)); +?> +--EXPECT-- +bool(false) From ca0414e64d7d37beff40c0c416bf83a3a858f2d2 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Wed, 12 Feb 2025 18:42:57 -0800 Subject: [PATCH 37/58] Reflection: show the type of object constants used as default properties When a property default is based on a global constant, show the type of the default. Previously, `format_default_value()` assumed that non-scalar and non-array defaults were always going to be `IS_CONSTANT_AST` pointers, and when the AST expression had been evaluated and produced an object, depending on when the `ReflectionClass` or `ReflectionProperty` instance had been created, the default was shown as one of `callable` or `__CLASS__`. Instead, if the default value is an object (`IS_OBJECT`), show the type of that object. Add test cases for both of the `callable` and `__CLASS__` cases to confirm that they now properly show the type of the constant. Closes GH-15902. Closes GH-17781. --- NEWS | 4 ++ ext/reflection/php_reflection.c | 17 ++++++--- .../gh15902/ReflectionClass-callable.phpt | 37 ++++++++++++++++++ .../tests/gh15902/ReflectionClass-class.phpt | 38 +++++++++++++++++++ .../gh15902/ReflectionProperty-callable.phpt | 20 ++++++++++ .../gh15902/ReflectionProperty-class.phpt | 20 ++++++++++ 6 files changed, 131 insertions(+), 5 deletions(-) create mode 100644 ext/reflection/tests/gh15902/ReflectionClass-callable.phpt create mode 100644 ext/reflection/tests/gh15902/ReflectionClass-class.phpt create mode 100644 ext/reflection/tests/gh15902/ReflectionProperty-callable.phpt create mode 100644 ext/reflection/tests/gh15902/ReflectionProperty-class.phpt diff --git a/NEWS b/NEWS index af80e9a4bb61c..1f7febd25da1e 100644 --- a/NEWS +++ b/NEWS @@ -51,6 +51,10 @@ PHP NEWS . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) . Fix memory leak in phpdbg calling registered function. (nielsdos) +- Reflection: + . Fixed bug GH-15902 (Core dumped in ext/reflection/php_reflection.c). + (DanielEScherzer) + - Streams: . Fixed bug GH-17650 (realloc with size 0 in user_filters.c). (nielsdos) . Fix memory leak on overflow in _php_stream_scandir(). (nielsdos) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 8cecc1083391e..50dcd489c8709 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -649,13 +649,20 @@ static int format_default_value(smart_str *str, zval *value) { } ZEND_HASH_FOREACH_END(); smart_str_appendc(str, ']'); } else if (Z_TYPE_P(value) == IS_OBJECT) { - /* This branch may only be reached for default properties, which don't support arbitrary objects. */ + /* This branch is reached if the constant AST was already evaluated and + * resulted in an object; show enum names, or the type of non-enums + * (GH-15902) */ zend_object *obj = Z_OBJ_P(value); zend_class_entry *class = obj->ce; - ZEND_ASSERT(class->ce_flags & ZEND_ACC_ENUM); - smart_str_append(str, class->name); - smart_str_appends(str, "::"); - smart_str_append(str, Z_STR_P(zend_enum_fetch_case_name(obj))); + if (class->ce_flags & ZEND_ACC_ENUM) { + smart_str_append(str, class->name); + smart_str_appends(str, "::"); + smart_str_append(str, Z_STR_P(zend_enum_fetch_case_name(obj))); + } else { + smart_str_appends(str, "object("); + smart_str_append(str, class->name); + smart_str_appends(str, ")"); + } } else { ZEND_ASSERT(Z_TYPE_P(value) == IS_CONSTANT_AST); zend_string *ast_str = zend_ast_export("", Z_ASTVAL_P(value), ""); diff --git a/ext/reflection/tests/gh15902/ReflectionClass-callable.phpt b/ext/reflection/tests/gh15902/ReflectionClass-callable.phpt new file mode 100644 index 0000000000000..0ed02dd837c9a --- /dev/null +++ b/ext/reflection/tests/gh15902/ReflectionClass-callable.phpt @@ -0,0 +1,37 @@ +--TEST-- +ReflectionClass object default property - used to say "callable" +--INI-- +opcache.enable_cli=0 +--FILE-- + +--EXPECTF-- +Class [ class C ] { + @@ %sReflectionClass-callable.php %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [1] { + Property [ public stdClass $a = object(stdClass) ] + } + + - Methods [0] { + } +} diff --git a/ext/reflection/tests/gh15902/ReflectionClass-class.phpt b/ext/reflection/tests/gh15902/ReflectionClass-class.phpt new file mode 100644 index 0000000000000..cb03cdd383fec --- /dev/null +++ b/ext/reflection/tests/gh15902/ReflectionClass-class.phpt @@ -0,0 +1,38 @@ +--TEST-- +ReflectionClass object default property - used to say "__CLASS__" +--INI-- +opcache.enable_cli=0 +--FILE-- + +--EXPECTF-- +Class [ class C ] { + @@ %sReflectionClass-class.php %d-%d + + - Constants [0] { + } + + - Static properties [0] { + } + + - Static methods [0] { + } + + - Properties [1] { + Property [ public stdClass $a = object(stdClass) ] + } + + - Methods [0] { + } +} diff --git a/ext/reflection/tests/gh15902/ReflectionProperty-callable.phpt b/ext/reflection/tests/gh15902/ReflectionProperty-callable.phpt new file mode 100644 index 0000000000000..b88d52721fce0 --- /dev/null +++ b/ext/reflection/tests/gh15902/ReflectionProperty-callable.phpt @@ -0,0 +1,20 @@ +--TEST-- +ReflectionProperty object default - used to say "callable" +--INI-- +opcache.enable_cli=0 +--FILE-- + +--EXPECTF-- +Property [ public stdClass $a = object(stdClass) ] diff --git a/ext/reflection/tests/gh15902/ReflectionProperty-class.phpt b/ext/reflection/tests/gh15902/ReflectionProperty-class.phpt new file mode 100644 index 0000000000000..021df4fe87be2 --- /dev/null +++ b/ext/reflection/tests/gh15902/ReflectionProperty-class.phpt @@ -0,0 +1,20 @@ +--TEST-- +ReflectionProperty object default - used to say "__CLASS__" +--INI-- +opcache.enable_cli=0 +--FILE-- + +--EXPECTF-- +Property [ public stdClass $a = object(stdClass) ] From 200f16fcf73f3e29c11d4700ad4afde1cc1aea63 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 18 Feb 2025 19:43:33 +0100 Subject: [PATCH 38/58] Fix GH-17855: CURL_STATICLIB flag set even if linked with shared lib We must define `CURL_STATICLIB` only when building against a static libcurl. The detection relies on our usual naming conventions, what should be revised in the future (possibly using pkg-config, or switching to CMake). Closes GH-17857. --- NEWS | 2 ++ ext/curl/config.w32 | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 1f7febd25da1e..a26321e6ca7d7 100644 --- a/NEWS +++ b/NEWS @@ -61,6 +61,8 @@ PHP NEWS - Windows: . Fixed phpize for Windows 11 (24H2). (bwoebi) + . Fixed GH-17855 (CURL_STATICLIB flag set even if linked with shared lib). + (cmb) - Zlib: . Fixed bug GH-17745 (zlib extension incorrectly handles object arguments). diff --git a/ext/curl/config.w32 b/ext/curl/config.w32 index f722c5faca5e3..65f3050f73a8f 100644 --- a/ext/curl/config.w32 +++ b/ext/curl/config.w32 @@ -13,7 +13,8 @@ if (PHP_CURL != "no") { } } - if (CHECK_LIB("libcurl_a.lib;libcurl.lib", "curl", PHP_CURL) && + var curl_location; + if ((curl_location = CHECK_LIB("libcurl_a.lib;libcurl.lib", "curl", PHP_CURL)) && CHECK_HEADER_ADD_INCLUDE("curl/easy.h", "CFLAGS_CURL") && SETUP_OPENSSL("curl", PHP_CURL) > 0 && CHECK_LIB("winmm.lib", "curl", PHP_CURL) && @@ -28,7 +29,10 @@ if (PHP_CURL != "no") { ) { EXTENSION("curl", "interface.c multi.c share.c curl_file.c"); AC_DEFINE('HAVE_CURL', 1, 'Have cURL library'); - ADD_FLAG("CFLAGS_CURL", "/D CURL_STATICLIB /D PHP_CURL_EXPORTS=1"); + ADD_FLAG("CFLAGS_CURL", "/D PHP_CURL_EXPORTS=1"); + if (curl_location.match(/libcurl_a\.lib$/)) { + ADD_FLAG("CFLAGS_CURL", "/D CURL_STATICLIB"); + } PHP_INSTALL_HEADERS("ext/curl", "php_curl.h"); // TODO: check for curl_version_info } else { From 9becccef17fcff691c8f7ee8491de6bfd25fc0d7 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 21 Feb 2025 19:25:35 +0100 Subject: [PATCH 39/58] Fix GH-17847: xinclude destroys live node dom_xinclude_strip_fallback_references() now also takes into account xi:include nodes children. This now subsumes all work done normally by the old start/end node removal, so we can remove that code and start using XML_PARSE_NOXINCNODE. Closes GH-17878. --- NEWS | 3 ++ ext/dom/document.c | 65 ++++---------------------------------- ext/dom/tests/gh17847.phpt | 49 ++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 58 deletions(-) create mode 100644 ext/dom/tests/gh17847.phpt diff --git a/NEWS b/NEWS index a26321e6ca7d7..2f542eef11ac4 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,9 @@ PHP NEWS . Fixed bug GH-17797 (zend_test_compile_string crash on invalid script path). (David Carlier) +- DOM: + . Fixed bug GH-17847 (xinclude destroys live node). (nielsdos) + - FFI: . Fix FFI Parsing of Pointer Declaration Lists. (davnotdev) diff --git a/ext/dom/document.c b/ext/dom/document.c index d1437f1f42923..0388249766e1f 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -1586,47 +1586,6 @@ PHP_METHOD(DOMDocument, saveXML) } /* }}} end dom_document_savexml */ -static xmlNodePtr php_dom_free_xinclude_node(xmlNodePtr cur) /* {{{ */ -{ - xmlNodePtr xincnode; - - xincnode = cur; - cur = cur->next; - xmlUnlinkNode(xincnode); - php_libxml_node_free_resource(xincnode); - - return cur; -} -/* }}} */ - -static void php_dom_remove_xinclude_nodes(xmlNodePtr cur) /* {{{ */ -{ - while(cur) { - if (cur->type == XML_XINCLUDE_START) { - cur = php_dom_free_xinclude_node(cur); - - /* XML_XINCLUDE_END node will be a sibling of XML_XINCLUDE_START */ - while(cur && cur->type != XML_XINCLUDE_END) { - /* remove xinclude processing nodes from recursive xincludes */ - if (cur->type == XML_ELEMENT_NODE) { - php_dom_remove_xinclude_nodes(cur->children); - } - cur = cur->next; - } - - if (cur && cur->type == XML_XINCLUDE_END) { - cur = php_dom_free_xinclude_node(cur); - } - } else { - if (cur->type == XML_ELEMENT_NODE) { - php_dom_remove_xinclude_nodes(cur->children); - } - cur = cur->next; - } - } -} -/* }}} */ - /* Backported from master branch xml_common.h */ static zend_always_inline xmlNodePtr php_dom_next_in_tree_order(const xmlNode *nodep, const xmlNode *basep) { @@ -1660,17 +1619,19 @@ static void dom_xinclude_strip_references(xmlNodePtr basep) } } -/* See GH-14702. - * We have to remove userland references to xinclude fallback nodes because libxml2 will make clones of these +/* See GH-14702 and GH-17847. + * We have to remove userland references to xinclude nodes because libxml2 will make clones of these * and remove the original nodes. If the originals are removed while there are still userland references * this will cause memory corruption. */ static void dom_xinclude_strip_fallback_references(const xmlNode *basep) { xmlNodePtr current = basep->children; + /* TODO: try to improve loop search performance */ while (current) { - if (current->type == XML_ELEMENT_NODE && current->ns != NULL && current->_private != NULL - && xmlStrEqual(current->name, XINCLUDE_FALLBACK) + if (current->type == XML_ELEMENT_NODE + && current->ns != NULL + && xmlStrEqual(current->name, XINCLUDE_NODE) && (xmlStrEqual(current->ns->href, XINCLUDE_NS) || xmlStrEqual(current->ns->href, XINCLUDE_OLD_NS))) { dom_xinclude_strip_references(current); } @@ -1684,7 +1645,6 @@ PHP_METHOD(DOMDocument, xinclude) { zval *id; xmlDoc *docp; - xmlNodePtr root; zend_long flags = 0; int err; dom_object *intern; @@ -1703,22 +1663,11 @@ PHP_METHOD(DOMDocument, xinclude) dom_xinclude_strip_fallback_references((const xmlNode *) docp); + flags |= XML_PARSE_NOXINCNODE; PHP_LIBXML_SANITIZE_GLOBALS(xinclude); err = xmlXIncludeProcessFlags(docp, (int)flags); PHP_LIBXML_RESTORE_GLOBALS(xinclude); - /* XML_XINCLUDE_START and XML_XINCLUDE_END nodes need to be removed as these - are added via xmlXIncludeProcess to mark beginning and ending of xincluded document - but are not wanted in resulting document - must be done even if err as it could fail after - having processed some xincludes */ - root = (xmlNodePtr) docp->children; - while(root && root->type != XML_ELEMENT_NODE && root->type != XML_XINCLUDE_START) { - root = root->next; - } - if (root) { - php_dom_remove_xinclude_nodes(root); - } - php_libxml_invalidate_node_list_cache(intern->document); if (err) { diff --git a/ext/dom/tests/gh17847.phpt b/ext/dom/tests/gh17847.phpt new file mode 100644 index 0000000000000..5d5df0b3be05f --- /dev/null +++ b/ext/dom/tests/gh17847.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-17847 (xinclude destroys live node) +--EXTENSIONS-- +dom +--FILE-- +loadXML(<< + + fallback

garbage

 +

garbage

+ + + +

garbage

+ + + +XML); + +$xpath = new DOMXPath($doc); + +$garbage = []; +foreach ($xpath->query('//p') as $entry) + $garbage[] = $entry; + +@$doc->xinclude(); + +var_dump($garbage); +?> +--EXPECT-- +array(3) { + [0]=> + object(DOMElement)#3 (1) { + ["schemaTypeInfo"]=> + NULL + } + [1]=> + object(DOMElement)#4 (1) { + ["schemaTypeInfo"]=> + NULL + } + [2]=> + object(DOMElement)#5 (1) { + ["schemaTypeInfo"]=> + NULL + } +} From 769f292a7a8fb95f17dc75c8c445f8ce0804fa75 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 21 Feb 2025 22:35:03 +0100 Subject: [PATCH 40/58] Fix GH-17879: readfile_variation8-win32.phpt test conflict Apparently a copy and paste issue. Closes GH-17881. --- ext/standard/tests/file/readfile_variation9.phpt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/standard/tests/file/readfile_variation9.phpt b/ext/standard/tests/file/readfile_variation9.phpt index a09846587a6dd..382186fed10ba 100644 --- a/ext/standard/tests/file/readfile_variation9.phpt +++ b/ext/standard/tests/file/readfile_variation9.phpt @@ -5,8 +5,8 @@ Dave Kelsey --FILE-- Date: Sun, 23 Feb 2025 09:36:05 +0000 Subject: [PATCH 41/58] Fix GH-17899: zend_test_compile_string crash on invalid script path. when opcache is enabled. close GH-17901 --- NEWS | 2 ++ ext/opcache/ZendAccelerator.c | 7 ++++++- ext/zend_test/tests/gh17899.phpt | 34 ++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 ext/zend_test/tests/gh17899.phpt diff --git a/NEWS b/NEWS index 2f542eef11ac4..71711936262d8 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,8 @@ PHP NEWS . Fixed bug GH-17654 (Multiple classes using same trait causes function JIT crash). (nielsdos) . Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry) + . Fixed bug GH-17899 (zend_test_compile_string with invalid path + when opcache is enabled). (David Carlier) - PDO_SQLite: . Fixed GH-17837 ()::getColumnMeta() on unexecuted statement segfaults). diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index a71a512c03edd..eac5cbbc41f7d 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -1334,7 +1334,12 @@ zend_string *accel_make_persistent_key(zend_string *str) EXPECTED((parent_script = zend_get_executed_filename_ex()) != NULL)) { parent_script_len = ZSTR_LEN(parent_script); - while ((--parent_script_len > 0) && !IS_SLASH(ZSTR_VAL(parent_script)[parent_script_len])); + while (parent_script_len > 0) { + --parent_script_len; + if (IS_SLASH(ZSTR_VAL(parent_script)[parent_script_len])) { + break; + } + } if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= ZCG_KEY_LEN)) { return NULL; diff --git a/ext/zend_test/tests/gh17899.phpt b/ext/zend_test/tests/gh17899.phpt new file mode 100644 index 0000000000000..6286243046047 --- /dev/null +++ b/ext/zend_test/tests/gh17899.phpt @@ -0,0 +1,34 @@ +--TEST-- +GH-17899 (zend_test_compile_string with opcache crash on invalid script path) +--EXTENSIONS-- +zend_test +--INI-- +opcache.enable_cli=1 +--CREDITS-- +YuanchengJiang +--FILE-- +'; +try {zend_test_compile_string($source,$source,$c);} catch (Exception $e) { echo($e); } +--EXPECTF-- + +Warning: Undefined variable $c in %s on line %d + +Deprecated: zend_test_compile_string(): Passing null to parameter #3 ($position) of type int is deprecated in %s on line %d + +Warning: require(sumfile.php): Failed to open stream: No such file or directory in on line %d + +Fatal error: Uncaught Error: Failed opening required 'sumfile.php' (include_path='.%s') in :%d +Stack trace: +#0 %s(%d): zend_test_compile_string(' on line %d + From 2c251f945cea63cf140d1669c7211e75e8a6cb00 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 23 Feb 2025 13:23:26 +0000 Subject: [PATCH 42/58] [skip ci] zend_test adding closing tags to newer tests --- ext/zend_test/tests/gh17797.phpt | 1 + ext/zend_test/tests/gh17899.phpt | 1 + 2 files changed, 2 insertions(+) diff --git a/ext/zend_test/tests/gh17797.phpt b/ext/zend_test/tests/gh17797.phpt index 9ae1bedb67425..271841b4389db 100644 --- a/ext/zend_test/tests/gh17797.phpt +++ b/ext/zend_test/tests/gh17797.phpt @@ -10,6 +10,7 @@ $source = ''; try {zend_test_compile_string($source,$source,$c);} catch (Exception $e) { echo($e); } +?> --EXPECTF-- Warning: Undefined variable $c in %s on line %d diff --git a/ext/zend_test/tests/gh17899.phpt b/ext/zend_test/tests/gh17899.phpt index 6286243046047..184fe0b2aa39d 100644 --- a/ext/zend_test/tests/gh17899.phpt +++ b/ext/zend_test/tests/gh17899.phpt @@ -12,6 +12,7 @@ $source = ''; try {zend_test_compile_string($source,$source,$c);} catch (Exception $e) { echo($e); } +?> --EXPECTF-- Warning: Undefined variable $c in %s on line %d From 353f21487f4dbdfbf80cb818d83787b013443550 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 23 Feb 2025 14:15:40 +0100 Subject: [PATCH 43/58] Fix cycle leak in sqlite3 setAuthorizer() Closes GH-17903. --- NEWS | 1 + ext/sqlite3/sqlite3.c | 4 +++- .../tests/setauthorizer_cycle_leak.phpt | 21 +++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 ext/sqlite3/tests/setauthorizer_cycle_leak.phpt diff --git a/NEWS b/NEWS index 71711936262d8..e587a7c770b29 100644 --- a/NEWS +++ b/NEWS @@ -48,6 +48,7 @@ PHP NEWS - PDO_SQLite: . Fixed GH-17837 ()::getColumnMeta() on unexecuted statement segfaults). (cmb) + . Fix cycle leak in sqlite3 setAuthorizer(). (nielsdos) - Phar: . Fixed bug GH-17808: PharFileInfo refcount bug. (nielsdos) diff --git a/ext/sqlite3/sqlite3.c b/ext/sqlite3/sqlite3.c index badcfcc29b0c5..09cb57410c87f 100644 --- a/ext/sqlite3/sqlite3.c +++ b/ext/sqlite3/sqlite3.c @@ -2256,7 +2256,7 @@ static HashTable *php_sqlite3_get_gc(zend_object *object, zval **table, int *n) { php_sqlite3_db_object *intern = php_sqlite3_db_from_obj(object); - if (intern->funcs == NULL && intern->collations == NULL) { + if (intern->funcs == NULL && intern->collations == NULL && !ZEND_FCC_INITIALIZED(intern->authorizer_fcc)) { /* Fast path without allocations */ *table = NULL; *n = 0; @@ -2264,6 +2264,8 @@ static HashTable *php_sqlite3_get_gc(zend_object *object, zval **table, int *n) } else { zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); + php_sqlite3_gc_buffer_add_fcc(gc_buffer, &intern->authorizer_fcc); + php_sqlite3_func *func = intern->funcs; while (func != NULL) { php_sqlite3_gc_buffer_add_fcc(gc_buffer, &func->func); diff --git a/ext/sqlite3/tests/setauthorizer_cycle_leak.phpt b/ext/sqlite3/tests/setauthorizer_cycle_leak.phpt new file mode 100644 index 0000000000000..b0ff384c72273 --- /dev/null +++ b/ext/sqlite3/tests/setauthorizer_cycle_leak.phpt @@ -0,0 +1,21 @@ +--TEST-- +setAuthorizer() cycle leak +--EXTENSIONS-- +sqlite3 +--FILE-- +setAuthorizer([$this, "foo"]); + } + + public function foo() {} +} + +$test = new Foo; + +echo "Done\n"; +?> +--EXPECT-- +Done From 065b4ec1255affac290473f8980a24092f4e3324 Mon Sep 17 00:00:00 2001 From: Michael Orlitzky Date: Wed, 23 Aug 2023 18:42:17 -0400 Subject: [PATCH 44/58] ext/gd/tests: backport optional PNG support - Three of our gd tests could be skipped with a message about requiring bundled GD, but those tests don't actually require bundled GD. We update the messages to mention the specific functions that are required. - add SKIPIF stanzas for missing PNG support The bundled libgd always has PNG support, but an external one may not. - imagerotate() is always available Following 59ec80c5, the imagerotate() function is always available. We may therefore remove its function_exists() checks without harm. close GH-17894 --- ext/gd/tests/bug22544-mb.phpt | 6 ++++++ ext/gd/tests/bug22544.phpt | 6 ++++++ ext/gd/tests/bug24155.phpt | 4 +++- ext/gd/tests/bug27582_1.phpt | 6 ++++++ ext/gd/tests/bug39366.phpt | 4 ---- ext/gd/tests/bug39780_extern.phpt | 3 +++ ext/gd/tests/bug43073.phpt | 3 +++ ext/gd/tests/bug43475.phpt | 3 +++ ext/gd/tests/bug43828.phpt | 3 +++ ext/gd/tests/bug45799.phpt | 6 ++++++ ext/gd/tests/bug47946.phpt | 3 +++ ext/gd/tests/bug48732-mb.phpt | 3 +++ ext/gd/tests/bug48732.phpt | 4 ++++ ext/gd/tests/bug50194.phpt | 4 +++- ext/gd/tests/bug51498.phpt | 3 +++ ext/gd/tests/bug52070.phpt | 6 ++++++ ext/gd/tests/bug53504.phpt | 3 +++ ext/gd/tests/bug64641.phpt | 3 +++ ext/gd/tests/bug66005.phpt | 6 ++++++ ext/gd/tests/bug72482_2.phpt | 6 ++++++ ext/gd/tests/bug72604.phpt | 6 ++++++ ext/gd/tests/bug72913.phpt | 6 ++++++ ext/gd/tests/bug73213.phpt | 6 ++++++ ext/gd/tests/bug73272.phpt | 6 ++++++ ext/gd/tests/bug73549.phpt | 6 ++++++ ext/gd/tests/bug73614.phpt | 3 +++ ext/gd/tests/bug74031.phpt | 6 ++++++ ext/gd/tests/bug75124.phpt | 3 +++ ext/gd/tests/bug77943.phpt | 6 ++++++ ext/gd/tests/bug79945.phpt | 3 +++ ext/gd/tests/imagearc_basic.phpt | 6 ++++++ ext/gd/tests/imagearc_variation1.phpt | 6 ++++++ ext/gd/tests/imagearc_variation2.phpt | 6 ++++++ ext/gd/tests/imagechar_basic.phpt | 6 ++++++ ext/gd/tests/imagecharup_basic.phpt | 6 ++++++ ext/gd/tests/imagecolorallocatealpha_basic.phpt | 3 +++ ext/gd/tests/imagecolorset_basic.phpt | 6 ++++++ ext/gd/tests/imageconvolution_basic.phpt | 6 ++++++ ext/gd/tests/imagecopyresampled_basic.phpt | 6 ++++++ ext/gd/tests/imagecreatefrombmp_basic.phpt | 3 +++ ext/gd/tests/imagecreatefromstring_bmp.phpt | 3 +++ ext/gd/tests/imagecreatefromtga_basic.phpt | 3 +++ ext/gd/tests/imagecreatefromtga_variation.phpt | 3 +++ ext/gd/tests/imagecreatetruecolor_basic.phpt | 3 +++ ext/gd/tests/imagecrop_auto.phpt | 3 +++ ext/gd/tests/imagedashedline_basic.phpt | 3 +++ ext/gd/tests/imageellipse_basic.phpt | 6 ++++++ ext/gd/tests/imagefilledarc_basic.phpt | 3 +++ ext/gd/tests/imagefilledarc_variation1.phpt | 3 +++ ext/gd/tests/imagefilledarc_variation2.phpt | 3 +++ ext/gd/tests/imagefilledellipse_basic.phpt | 6 ++++++ ext/gd/tests/imagefilledpolygon_basic.phpt | 3 +++ ext/gd/tests/imagefilltoborder_basic.phpt | 6 ++++++ ext/gd/tests/imagefilter.phpt | 5 ++++- ext/gd/tests/imagegammacorrect_basic.phpt | 3 +++ ext/gd/tests/imagegammacorrect_variation1.phpt | 3 +++ ext/gd/tests/imagegammacorrect_variation2.phpt | 6 ++++++ ext/gd/tests/imageopenpolygon_basic.phpt | 6 ++++++ ext/gd/tests/imagepolygon_aa.phpt | 6 ++++++ ext/gd/tests/imagepolygon_basic.phpt | 3 +++ ext/gd/tests/imagerectangle_basic.phpt | 6 ++++++ ext/gd/tests/imageresolution_png.phpt | 6 ++++++ ext/gd/tests/imagerotate_overflow.phpt | 6 ------ ext/gd/tests/imagesetbrush_basic.phpt | 6 ++++++ ext/gd/tests/imagesetthickness_basic.phpt | 3 +++ ext/gd/tests/imagestring_basic.phpt | 6 ++++++ ext/gd/tests/imagestringup_basic.phpt | 6 ++++++ ext/gd/tests/imagetruecolortopalette_basic.phpt | 3 +++ ext/gd/tests/libgd00086_extern.phpt | 3 +++ ext/gd/tests/test_image_equals_file_palette.phpt | 6 ++++++ 70 files changed, 308 insertions(+), 13 deletions(-) diff --git a/ext/gd/tests/bug22544-mb.phpt b/ext/gd/tests/bug22544-mb.phpt index b67478e558c47..f99307df1212b 100644 --- a/ext/gd/tests/bug22544-mb.phpt +++ b/ext/gd/tests/bug22544-mb.phpt @@ -2,6 +2,12 @@ Bug #22544 (TrueColor transparency in PNG images). --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- --FILE-- --FILE-- 45) --EXTENSIONS-- gd ---SKIPIF-- - --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- = 2.2.5'); } +if (!(imagetypes() & IMG_PNG)) { + die("skip No PNG support"); +} ?> --FILE-- --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- #testfest PHPSP on 2009-06-30 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- --FILE-- #testfest PHPSP on 2009-06-30 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- #testfest PHPSP on 2009-06-20 --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- --FILE-- --FILE-- --FILE-- Date: Sun, 23 Feb 2025 22:35:55 -0500 Subject: [PATCH 45/58] Fix GH-17891 gh17373.phpt test issue without freetype support skip if imagefttext() is not available This test calls imagefttext(), which may not be available if libgd was built without freetype support. Closes GH-17910 --- ext/gd/tests/gh17373.phpt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ext/gd/tests/gh17373.phpt b/ext/gd/tests/gh17373.phpt index 354cdd07362b2..f62c74fbd3e05 100644 --- a/ext/gd/tests/gh17373.phpt +++ b/ext/gd/tests/gh17373.phpt @@ -2,6 +2,10 @@ Bug GH-17373 (imagefttext() ignores clipping rect for palette images) --EXTENSIONS-- gd +--SKIPIF-- + --FILE-- Date: Mon, 24 Feb 2025 12:20:34 +0100 Subject: [PATCH 46/58] [skip ci] Use laravel default branch in community build See: https://github.com/laravel/framework/issues/54754#issuecomment-2678092563 According to this comment, the default branch more actively receives changes throuought the year than master. Hence, it makes more sense to test the default branch. --- .github/workflows/nightly.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4c8ec23b158a4..2377286ca830c 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -520,7 +520,7 @@ jobs: - name: Test Laravel if: ${{ !cancelled() }} run: | - git clone https://github.com/laravel/framework.git --branch=master --depth=1 + git clone https://github.com/laravel/framework.git --depth=1 cd framework git rev-parse HEAD php /usr/bin/composer install --no-progress --ignore-platform-reqs From 3b4a58da44ed90adb11a98b88aea9114bbca14b1 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Mon, 24 Feb 2025 19:52:57 +0100 Subject: [PATCH 47/58] Backport GH-17869 to PHP 8.3 JIT Closes GH-17918. --- NEWS | 1 + ext/opcache/jit/zend_jit_arm64.dasc | 17 ++++++++++++----- ext/opcache/jit/zend_jit_x86.dasc | 16 +++++++++++----- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index e587a7c770b29..1f6ba6a9bafff 100644 --- a/NEWS +++ b/NEWS @@ -44,6 +44,7 @@ PHP NEWS . Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry) . Fixed bug GH-17899 (zend_test_compile_string with invalid path when opcache is enabled). (David Carlier) + . Fixed bug GH-17868 (Cannot allocate memory with tracing JIT). (nielsdos) - PDO_SQLite: . Fixed GH-17837 ()::getColumnMeta() on unexecuted statement segfaults). diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index ec6fae8819fcd..985c08f0512ea 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -8655,11 +8655,18 @@ static int zend_jit_push_call_frame(dasm_State **Dst, const zend_op *opline, con | str TMP1w, EX:RX->This.u1.type_info | // Z_PTR(call->This) = object_or_called_scope; | str REG1, EX:RX->This.value.ptr - | ldr TMP1, [REG0, #offsetof(zend_closure, func.op_array.run_time_cache__ptr)] - | cbnz TMP1, >1 - | add FCARG1x, REG0, #offsetof(zend_closure, func) - | EXT_CALL zend_jit_init_func_run_time_cache_helper, REG0 - |1: + if (!func) { + | ldrb TMP1w, [REG0, #offsetof(zend_closure, func.type)] + | cmp TMP1w, #ZEND_USER_FUNCTION + | beq >1 + } + if (!func || func->common.type == ZEND_USER_FUNCTION) { + | add FCARG1x, REG0, #offsetof(zend_closure, func) + | EXT_CALL zend_jit_init_func_run_time_cache_helper, REG0 + } + if (!func) { + |1: + } } | // ZEND_CALL_NUM_ARGS(call) = num_args; | LOAD_32BIT_VAL TMP1w, opline->extended_value diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index f65dc769db57d..1f1abb59a1c24 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -9255,11 +9255,17 @@ static int zend_jit_push_call_frame(dasm_State **Dst, const zend_op *opline, con | or dword EX:RX->This.u1.type_info, edx | // Z_PTR(call->This) = object_or_called_scope; | mov aword EX:RX->This.value.ptr, r1 - | cmp aword [r0 + offsetof(zend_closure, func.op_array.run_time_cache__ptr)], 0 - | jnz >1 - | lea FCARG1a, aword [r0 + offsetof(zend_closure, func)] - | EXT_CALL zend_jit_init_func_run_time_cache_helper, r0 - |1: + if (!func) { + | cmp byte [r0 + offsetof(zend_closure, func.type)], ZEND_USER_FUNCTION + | jnz >1 + } + if (!func || func->common.type == ZEND_USER_FUNCTION) { + | lea FCARG1a, aword [r0 + offsetof(zend_closure, func)] + | EXT_CALL zend_jit_init_func_run_time_cache_helper, r0 + } + if (!func) { + |1: + } } | // ZEND_CALL_NUM_ARGS(call) = num_args; | mov dword EX:RX->This.u2.num_args, opline->extended_value From 930624899bb996efc2f6a24b992ede90c93c8902 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 25 Jan 2025 23:08:27 +0100 Subject: [PATCH 48/58] Fix bug #72666: stat cache not cleared for plain paths This adds more aggressive clearing of stat cache. It is added to the filestat as well as plain wrapper operations which covers stream file accessing as well as exec functions (using pipes). It should hopefully fix the most visible issues with the stat cache. Closes GH-17681 --- NEWS | 4 +++ ext/standard/filestat.c | 12 +++++++ .../tests/file/bug72666_variation1.phpt | 19 ++++++++++ .../tests/file/bug72666_variation2.phpt | 35 +++++++++++++++++++ .../tests/file/bug72666_variation3.phpt | 35 +++++++++++++++++++ .../tests/file/bug72666_variation4.phpt | 26 ++++++++++++++ main/streams/plain_wrapper.c | 31 +++++++++++++--- 7 files changed, 158 insertions(+), 4 deletions(-) create mode 100644 ext/standard/tests/file/bug72666_variation1.phpt create mode 100644 ext/standard/tests/file/bug72666_variation2.phpt create mode 100644 ext/standard/tests/file/bug72666_variation3.phpt create mode 100644 ext/standard/tests/file/bug72666_variation4.phpt diff --git a/NEWS b/NEWS index 1f6ba6a9bafff..e644f7c0358d4 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,10 @@ PHP NEWS . Fixed bug GH-15902 (Core dumped in ext/reflection/php_reflection.c). (DanielEScherzer) +- Standard: + . Fixed bug #72666 (stat cache clearing inconsistent between file:// paths + and plain paths). (Jakub Zelenka) + - Streams: . Fixed bug GH-17650 (realloc with size 0 in user_filters.c). (nielsdos) . Fix memory leak on overflow in _php_stream_scandir(). (nielsdos) diff --git a/ext/standard/filestat.c b/ext/standard/filestat.c index 1d83dcfe940df..9b8ff7efd6a66 100644 --- a/ext/standard/filestat.c +++ b/ext/standard/filestat.c @@ -388,6 +388,9 @@ static void php_do_chgrp(INTERNAL_FUNCTION_PARAMETERS, int do_lchgrp) /* {{{ */ php_error_docref(NULL, E_WARNING, "%s", strerror(errno)); RETURN_FALSE; } + + php_clear_stat_cache(0, NULL, 0); + RETURN_TRUE; #endif } @@ -527,6 +530,9 @@ static void php_do_chown(INTERNAL_FUNCTION_PARAMETERS, int do_lchown) /* {{{ */ php_error_docref(NULL, E_WARNING, "%s", strerror(errno)); RETURN_FALSE; } + + php_clear_stat_cache(0, NULL, 0); + RETURN_TRUE; #endif } @@ -591,6 +597,9 @@ PHP_FUNCTION(chmod) php_error_docref(NULL, E_WARNING, "%s", strerror(errno)); RETURN_FALSE; } + + php_clear_stat_cache(0, NULL, 0); + RETURN_TRUE; } /* }}} */ @@ -676,6 +685,9 @@ PHP_FUNCTION(touch) php_error_docref(NULL, E_WARNING, "Utime failed: %s", strerror(errno)); RETURN_FALSE; } + + php_clear_stat_cache(0, NULL, 0); + RETURN_TRUE; } /* }}} */ diff --git a/ext/standard/tests/file/bug72666_variation1.phpt b/ext/standard/tests/file/bug72666_variation1.phpt new file mode 100644 index 0000000000000..6e59405d14ca8 --- /dev/null +++ b/ext/standard/tests/file/bug72666_variation1.phpt @@ -0,0 +1,19 @@ +--TEST-- +Bug #72666 (stat cache clearing inconsistent - touch) +--FILE-- + 2); +touch($filename, 1); +var_dump(filemtime($filename)); + +?> +--CLEAN-- + +--EXPECT-- +bool(true) +int(1) diff --git a/ext/standard/tests/file/bug72666_variation2.phpt b/ext/standard/tests/file/bug72666_variation2.phpt new file mode 100644 index 0000000000000..7621133c71b24 --- /dev/null +++ b/ext/standard/tests/file/bug72666_variation2.phpt @@ -0,0 +1,35 @@ +--TEST-- +Bug #72666 (stat cache clearing inconsistent - chgrp, chmod) +--SKIPIF-- + +--FILE-- + $ctime1); +var_dump($ctime3 > $ctime2); +?> +--CLEAN-- + +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) diff --git a/ext/standard/tests/file/bug72666_variation3.phpt b/ext/standard/tests/file/bug72666_variation3.phpt new file mode 100644 index 0000000000000..a491640c4f746 --- /dev/null +++ b/ext/standard/tests/file/bug72666_variation3.phpt @@ -0,0 +1,35 @@ +--TEST-- +Bug #72666 (stat cache clearing inconsistent - plain wrapper) +--FILE-- + $atime1); +} +var_dump($mtime2 > $mtime1); +?> +--CLEAN-- + +--EXPECT-- +string(4) "test" +int(4) +bool(true) +bool(true) diff --git a/ext/standard/tests/file/bug72666_variation4.phpt b/ext/standard/tests/file/bug72666_variation4.phpt new file mode 100644 index 0000000000000..09e32dafed9cf --- /dev/null +++ b/ext/standard/tests/file/bug72666_variation4.phpt @@ -0,0 +1,26 @@ +--TEST-- +Bug #72666 (stat cache clearing inconsistent - exec) +--FILE-- + 1); + + +touch($filename, 1); +var_dump(filemtime($filename)); +shell_exec("touch $filename"); +var_dump(filemtime($filename) > 1); +?> +--CLEAN-- + +--EXPECT-- +int(1) +bool(true) +int(1) +bool(true) diff --git a/main/streams/plain_wrapper.c b/main/streams/plain_wrapper.c index 83a1b9fed49d5..1d5b7cfdac40d 100644 --- a/main/streams/plain_wrapper.c +++ b/main/streams/plain_wrapper.c @@ -349,14 +349,15 @@ PHPAPI php_stream *_php_stream_fopen_from_pipe(FILE *file, const char *mode STRE static ssize_t php_stdiop_write(php_stream *stream, const char *buf, size_t count) { php_stdio_stream_data *data = (php_stdio_stream_data*)stream->abstract; + ssize_t bytes_written; assert(data != NULL); if (data->fd >= 0) { #ifdef PHP_WIN32 - ssize_t bytes_written = _write(data->fd, buf, PLAIN_WRAP_BUF_SIZE(count)); + bytes_written = _write(data->fd, buf, PLAIN_WRAP_BUF_SIZE(count)); #else - ssize_t bytes_written = write(data->fd, buf, count); + bytes_written = write(data->fd, buf, count); #endif if (bytes_written < 0) { if (PHP_IS_TRANSIENT_ERROR(errno)) { @@ -370,7 +371,6 @@ static ssize_t php_stdiop_write(php_stream *stream, const char *buf, size_t coun php_error_docref(NULL, E_NOTICE, "Write of %zu bytes failed with errno=%d %s", count, errno, strerror(errno)); } } - return bytes_written; } else { #ifdef HAVE_FLUSHIO @@ -380,8 +380,15 @@ static ssize_t php_stdiop_write(php_stream *stream, const char *buf, size_t coun data->last_op = 'w'; #endif - return (ssize_t) fwrite(buf, 1, count, data->file); + bytes_written = (ssize_t) fwrite(buf, 1, count, data->file); } + + if (EG(active)) { + /* clear stat cache as mtime and ctime got changed */ + php_clear_stat_cache(0, NULL, 0); + } + + return bytes_written; } static ssize_t php_stdiop_read(php_stream *stream, char *buf, size_t count) @@ -460,6 +467,12 @@ static ssize_t php_stdiop_read(php_stream *stream, char *buf, size_t count) stream->eof = feof(data->file); } + + if (EG(active)) { + /* clear stat cache as atime got changed */ + php_clear_stat_cache(0, NULL, 0); + } + return ret; } @@ -540,6 +553,10 @@ static int php_stdiop_flush(php_stream *stream) * something completely different. */ if (data->file) { + if (EG(active)) { + /* clear stat cache as there might be a write so mtime and ctime might have changed */ + php_clear_stat_cache(0, NULL, 0); + } return fflush(data->file); } return 0; @@ -1154,6 +1171,12 @@ PHPAPI php_stream *_php_stream_fopen(const char *filename, const char *mode, zen ret = php_stream_fopen_from_fd_rel(fd, mode, persistent_id, (open_flags & O_APPEND) == 0); } + if (EG(active)) { + /* clear stat cache as mtime and ctime might got changed - phar can use stream before + * cache is initialized so we need to check if the execution is active. */ + php_clear_stat_cache(0, NULL, 0); + } + if (ret) { if (opened_path) { *opened_path = zend_string_init(realpath, strlen(realpath), 0); From bf30a4454d775a5a952d1a799dd24aac55b5133c Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 31 Dec 2024 18:57:02 +0100 Subject: [PATCH 49/58] Fix GHSA-ghsa-v8xr-gpvj-cx9g: http header folding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds HTTP header folding support for HTTP wrapper response headers. Reviewed-by: Tim Düsterhus --- ext/standard/http_fopen_wrapper.c | 365 ++++++++++++------ .../tests/http/ghsa-v8xr-gpvj-cx9g-001.phpt | 49 +++ .../tests/http/ghsa-v8xr-gpvj-cx9g-002.phpt | 51 +++ .../tests/http/ghsa-v8xr-gpvj-cx9g-003.phpt | 49 +++ .../tests/http/ghsa-v8xr-gpvj-cx9g-004.phpt | 48 +++ .../tests/http/ghsa-v8xr-gpvj-cx9g-005.phpt | 48 +++ .../tests/http/http_response_header_05.phpt | 30 -- 7 files changed, 495 insertions(+), 145 deletions(-) create mode 100644 ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-001.phpt create mode 100644 ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-002.phpt create mode 100644 ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-003.phpt create mode 100644 ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-004.phpt create mode 100644 ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-005.phpt delete mode 100644 ext/standard/tests/http/http_response_header_05.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index fb987c82e5cc2..a08aeedd11a29 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -115,6 +115,182 @@ static bool check_has_header(const char *headers, const char *header) { return 0; } +typedef struct _php_stream_http_response_header_info { + php_stream_filter *transfer_encoding; + size_t file_size; + bool follow_location; + char location[HTTP_HEADER_BLOCK_SIZE]; +} php_stream_http_response_header_info; + +static void php_stream_http_response_header_info_init( + php_stream_http_response_header_info *header_info) +{ + header_info->transfer_encoding = NULL; + header_info->file_size = 0; + header_info->follow_location = 1; + header_info->location[0] = '\0'; +} + +/* Trim white spaces from response header line and update its length */ +static bool php_stream_http_response_header_trim(char *http_header_line, + size_t *http_header_line_length) +{ + char *http_header_line_end = http_header_line + *http_header_line_length - 1; + while (http_header_line_end >= http_header_line && + (*http_header_line_end == '\n' || *http_header_line_end == '\r')) { + http_header_line_end--; + } + + /* The primary definition of an HTTP header in RFC 7230 states: + * > Each header field consists of a case-insensitive field name followed + * > by a colon (":"), optional leading whitespace, the field value, and + * > optional trailing whitespace. */ + + /* Strip trailing whitespace */ + bool space_trim = (*http_header_line_end == ' ' || *http_header_line_end == '\t'); + if (space_trim) { + do { + http_header_line_end--; + } while (http_header_line_end >= http_header_line && + (*http_header_line_end == ' ' || *http_header_line_end == '\t')); + } + http_header_line_end++; + *http_header_line_end = '\0'; + *http_header_line_length = http_header_line_end - http_header_line; + + return space_trim; +} + +/* Process folding headers of the current line and if there are none, parse last full response + * header line. It returns NULL if the last header is finished, otherwise it returns updated + * last header line. */ +static zend_string *php_stream_http_response_headers_parse(php_stream *stream, + php_stream_context *context, int options, zend_string *last_header_line_str, + char *header_line, size_t *header_line_length, int response_code, + zval *response_header, php_stream_http_response_header_info *header_info) +{ + char *last_header_line = ZSTR_VAL(last_header_line_str); + size_t last_header_line_length = ZSTR_LEN(last_header_line_str); + char *last_header_line_end = ZSTR_VAL(last_header_line_str) + ZSTR_LEN(last_header_line_str) - 1; + + /* Process non empty header line. */ + if (header_line && (*header_line != '\n' && *header_line != '\r')) { + /* Removing trailing white spaces. */ + if (php_stream_http_response_header_trim(header_line, header_line_length) && + *header_line_length == 0) { + /* Only spaces so treat as an empty folding header. */ + return last_header_line_str; + } + + /* Process folding headers if starting with a space or a tab. */ + if (header_line && (*header_line == ' ' || *header_line == '\t')) { + char *http_folded_header_line = header_line; + size_t http_folded_header_line_length = *header_line_length; + /* Remove the leading white spaces. */ + while (*http_folded_header_line == ' ' || *http_folded_header_line == '\t') { + http_folded_header_line++; + http_folded_header_line_length--; + } + /* It has to have some characters because it would get returned after the call + * php_stream_http_response_header_trim above. */ + ZEND_ASSERT(http_folded_header_line_length > 0); + /* Concatenate last header line, space and current header line. */ + zend_string *extended_header_str = zend_string_concat3( + last_header_line, last_header_line_length, + " ", 1, + http_folded_header_line, http_folded_header_line_length); + zend_string_efree(last_header_line_str); + last_header_line_str = extended_header_str; + /* Return new header line. */ + return last_header_line_str; + } + } + + /* Find header separator position. */ + char *last_header_value = memchr(last_header_line, ':', last_header_line_length); + if (last_header_value) { + last_header_value++; /* Skip ':'. */ + + /* Strip leading whitespace. */ + while (last_header_value < last_header_line_end + && (*last_header_value == ' ' || *last_header_value == '\t')) { + last_header_value++; + } + } else { + /* There is no colon. Set the value to the end of the header line, which is effectively + * an empty string. */ + last_header_value = last_header_line_end; + } + + bool store_header = true; + zval *tmpzval = NULL; + + if (!strncasecmp(last_header_line, "Location:", sizeof("Location:")-1)) { + /* Check if the location should be followed. */ + if (context && (tmpzval = php_stream_context_get_option(context, "http", "follow_location")) != NULL) { + header_info->follow_location = zval_is_true(tmpzval); + } else if (!((response_code >= 300 && response_code < 304) + || 307 == response_code || 308 == response_code)) { + /* The redirection should not be automatic if follow_location is not set and + * response_code not in (300, 301, 302, 303 and 307) + * see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.1 + * RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */ + header_info->follow_location = 0; + } + strlcpy(header_info->location, last_header_value, sizeof(header_info->location)); + } else if (!strncasecmp(last_header_line, "Content-Type:", sizeof("Content-Type:")-1)) { + php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, last_header_value, 0); + } else if (!strncasecmp(last_header_line, "Content-Length:", sizeof("Content-Length:")-1)) { + /* https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length */ + const char *ptr = last_header_value; + /* must contain only digits, no + or - symbols */ + if (*ptr >= '0' && *ptr <= '9') { + char *endptr = NULL; + size_t parsed = ZEND_STRTOUL(ptr, &endptr, 10); + /* check whether there was no garbage in the header value and the conversion was successful */ + if (endptr && !*endptr) { + /* truncate for 32-bit such that no negative file sizes occur */ + header_info->file_size = MIN(parsed, ZEND_LONG_MAX); + php_stream_notify_file_size(context, header_info->file_size, last_header_line, 0); + } + } + } else if ( + !strncasecmp(last_header_line, "Transfer-Encoding:", sizeof("Transfer-Encoding:")-1) + && !strncasecmp(last_header_value, "Chunked", sizeof("Chunked")-1) + ) { + /* Create filter to decode response body. */ + if (!(options & STREAM_ONLY_GET_HEADERS)) { + zend_long decode = 1; + + if (context && (tmpzval = php_stream_context_get_option(context, "http", "auto_decode")) != NULL) { + decode = zend_is_true(tmpzval); + } + if (decode) { + if (header_info->transfer_encoding != NULL) { + /* Prevent a memory leak in case there are more transfer-encoding headers. */ + php_stream_filter_free(header_info->transfer_encoding); + } + header_info->transfer_encoding = php_stream_filter_create( + "dechunk", NULL, php_stream_is_persistent(stream)); + if (header_info->transfer_encoding != NULL) { + /* Do not store transfer-encoding header. */ + store_header = false; + } + } + } + } + + if (store_header) { + zval http_header; + ZVAL_NEW_STR(&http_header, last_header_line_str); + zend_hash_next_index_insert(Z_ARRVAL_P(response_header), &http_header); + } else { + zend_string_efree(last_header_line_str); + } + + return NULL; +} + static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context, int redirect_max, int flags, @@ -127,11 +303,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, zend_string *tmp = NULL; char *ua_str = NULL; zval *ua_zval = NULL, *tmpzval = NULL, ssl_proxy_peer_name; - char location[HTTP_HEADER_BLOCK_SIZE]; int reqok = 0; char *http_header_line = NULL; + zend_string *last_header_line_str = NULL; + php_stream_http_response_header_info header_info; char tmp_line[128]; - size_t chunk_size = 0, file_size = 0; + size_t chunk_size = 0; int eol_detect = 0; zend_string *transport_string; zend_string *errstr = NULL; @@ -142,8 +319,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, int header_init = ((flags & HTTP_WRAPPER_HEADER_INIT) != 0); int redirected = ((flags & HTTP_WRAPPER_REDIRECTED) != 0); int redirect_keep_method = ((flags & HTTP_WRAPPER_KEEP_METHOD) != 0); - bool follow_location = 1; - php_stream_filter *transfer_encoding = NULL; int response_code; smart_str req_buf = {0}; bool custom_request_method; @@ -666,8 +841,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, /* send it */ php_stream_write(stream, ZSTR_VAL(req_buf.s), ZSTR_LEN(req_buf.s)); - location[0] = '\0'; - if (Z_ISUNDEF_P(response_header)) { array_init(response_header); } @@ -749,141 +922,101 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, } } - /* read past HTTP headers */ + php_stream_http_response_header_info_init(&header_info); + /* read past HTTP headers */ while (!php_stream_eof(stream)) { size_t http_header_line_length; if (http_header_line != NULL) { efree(http_header_line); } - if ((http_header_line = php_stream_get_line(stream, NULL, 0, &http_header_line_length)) && *http_header_line != '\n' && *http_header_line != '\r') { - char *e = http_header_line + http_header_line_length - 1; - char *http_header_value; - - while (e >= http_header_line && (*e == '\n' || *e == '\r')) { - e--; - } - - /* The primary definition of an HTTP header in RFC 7230 states: - * > Each header field consists of a case-insensitive field name followed - * > by a colon (":"), optional leading whitespace, the field value, and - * > optional trailing whitespace. */ - - /* Strip trailing whitespace */ - while (e >= http_header_line && (*e == ' ' || *e == '\t')) { - e--; - } - - /* Terminate header line */ - e++; - *e = '\0'; - http_header_line_length = e - http_header_line; - - http_header_value = memchr(http_header_line, ':', http_header_line_length); - if (http_header_value) { - http_header_value++; /* Skip ':' */ - - /* Strip leading whitespace */ - while (http_header_value < e - && (*http_header_value == ' ' || *http_header_value == '\t')) { - http_header_value++; + if ((http_header_line = php_stream_get_line(stream, NULL, 0, &http_header_line_length))) { + bool last_line; + if (*http_header_line == '\r') { + if (http_header_line[1] != '\n') { + php_stream_close(stream); + stream = NULL; + php_stream_wrapper_log_error(wrapper, options, + "HTTP invalid header name (cannot start with CR character)!"); + goto out; } + last_line = true; + } else if (*http_header_line == '\n') { + last_line = true; } else { - /* There is no colon. Set the value to the end of the header line, which is - * effectively an empty string. */ - http_header_value = e; + last_line = false; } - - if (!strncasecmp(http_header_line, "Location:", sizeof("Location:")-1)) { - if (context && (tmpzval = php_stream_context_get_option(context, "http", "follow_location")) != NULL) { - follow_location = zval_is_true(tmpzval); - } else if (!((response_code >= 300 && response_code < 304) - || 307 == response_code || 308 == response_code)) { - /* we shouldn't redirect automatically - if follow_location isn't set and response_code not in (300, 301, 302, 303 and 307) - see http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.3.1 - RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */ - follow_location = 0; + + if (last_header_line_str != NULL) { + /* Parse last header line. */ + last_header_line_str = php_stream_http_response_headers_parse(stream, context, + options, last_header_line_str, http_header_line, &http_header_line_length, + response_code, response_header, &header_info); + if (last_header_line_str != NULL) { + /* Folding header present so continue. */ + continue; } - strlcpy(location, http_header_value, sizeof(location)); - } else if (!strncasecmp(http_header_line, "Content-Type:", sizeof("Content-Type:")-1)) { - php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, http_header_value, 0); - } else if (!strncasecmp(http_header_line, "Content-Length:", sizeof("Content-Length:")-1)) { - /* https://www.rfc-editor.org/rfc/rfc9110.html#name-content-length */ - const char *ptr = http_header_value; - /* must contain only digits, no + or - symbols */ - if (*ptr >= '0' && *ptr <= '9') { - char *endptr = NULL; - size_t parsed = ZEND_STRTOUL(ptr, &endptr, 10); - /* check whether there was no garbage in the header value and the conversion was successful */ - if (endptr && !*endptr) { - /* truncate for 32-bit such that no negative file sizes occur */ - file_size = MIN(parsed, ZEND_LONG_MAX); - php_stream_notify_file_size(context, file_size, http_header_line, 0); - } - } - } else if ( - !strncasecmp(http_header_line, "Transfer-Encoding:", sizeof("Transfer-Encoding:")-1) - && !strncasecmp(http_header_value, "Chunked", sizeof("Chunked")-1) - ) { - - /* create filter to decode response body */ - if (!(options & STREAM_ONLY_GET_HEADERS)) { - zend_long decode = 1; - - if (context && (tmpzval = php_stream_context_get_option(context, "http", "auto_decode")) != NULL) { - decode = zend_is_true(tmpzval); - } - if (decode) { - transfer_encoding = php_stream_filter_create("dechunk", NULL, php_stream_is_persistent(stream)); - if (transfer_encoding) { - /* don't store transfer-encodeing header */ - continue; - } - } + } else if (!last_line) { + /* The first line cannot start with spaces. */ + if (*http_header_line == ' ' || *http_header_line == '\t') { + php_stream_close(stream); + stream = NULL; + php_stream_wrapper_log_error(wrapper, options, + "HTTP invalid response format (folding header at the start)!"); + goto out; } + /* Trim the first line if it is not the last line. */ + php_stream_http_response_header_trim(http_header_line, &http_header_line_length); } - - { - zval http_header; - ZVAL_STRINGL(&http_header, http_header_line, http_header_line_length); - zend_hash_next_index_insert(Z_ARRVAL_P(response_header), &http_header); + if (last_line) { + /* For the last line the last header line must be NULL. */ + ZEND_ASSERT(last_header_line_str == NULL); + break; } + /* Save current line as the last line so it gets parsed in the next round. */ + last_header_line_str = zend_string_init(http_header_line, http_header_line_length, 0); } else { break; } } - if (!reqok || (location[0] != '\0' && follow_location)) { - if (!follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) { + /* If the stream was closed early, we still want to process the last line to keep BC. */ + if (last_header_line_str != NULL) { + php_stream_http_response_headers_parse(stream, context, options, last_header_line_str, + NULL, NULL, response_code, response_header, &header_info); + } + + if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) { + if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) { goto out; } - if (location[0] != '\0') - php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, location, 0); + if (header_info.location[0] != '\0') + php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0); php_stream_close(stream); stream = NULL; - if (transfer_encoding) { - php_stream_filter_free(transfer_encoding); - transfer_encoding = NULL; + if (header_info.transfer_encoding) { + php_stream_filter_free(header_info.transfer_encoding); + header_info.transfer_encoding = NULL; } - if (location[0] != '\0') { + if (header_info.location[0] != '\0') { char new_path[HTTP_HEADER_BLOCK_SIZE]; char loc_path[HTTP_HEADER_BLOCK_SIZE]; *new_path='\0'; - if (strlen(location)<8 || (strncasecmp(location, "http://", sizeof("http://")-1) && - strncasecmp(location, "https://", sizeof("https://")-1) && - strncasecmp(location, "ftp://", sizeof("ftp://")-1) && - strncasecmp(location, "ftps://", sizeof("ftps://")-1))) + if (strlen(header_info.location) < 8 || + (strncasecmp(header_info.location, "http://", sizeof("http://")-1) && + strncasecmp(header_info.location, "https://", sizeof("https://")-1) && + strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) && + strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1))) { - if (*location != '/') { - if (*(location+1) != '\0' && resource->path) { + if (*header_info.location != '/') { + if (*(header_info.location+1) != '\0' && resource->path) { char *s = strrchr(ZSTR_VAL(resource->path), '/'); if (!s) { s = ZSTR_VAL(resource->path); @@ -899,15 +1032,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, if (resource->path && ZSTR_VAL(resource->path)[0] == '/' && ZSTR_VAL(resource->path)[1] == '\0') { - snprintf(loc_path, sizeof(loc_path) - 1, "%s%s", ZSTR_VAL(resource->path), location); + snprintf(loc_path, sizeof(loc_path) - 1, "%s%s", + ZSTR_VAL(resource->path), header_info.location); } else { - snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s", ZSTR_VAL(resource->path), location); + snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s", + ZSTR_VAL(resource->path), header_info.location); } } else { - snprintf(loc_path, sizeof(loc_path) - 1, "/%s", location); + snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location); } } else { - strlcpy(loc_path, location, sizeof(loc_path)); + strlcpy(loc_path, header_info.location, sizeof(loc_path)); } if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) { snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path); @@ -915,7 +1050,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path); } } else { - strlcpy(new_path, location, sizeof(new_path)); + strlcpy(new_path, header_info.location, sizeof(new_path)); } php_url_free(resource); @@ -975,7 +1110,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, if (header_init) { ZVAL_COPY(&stream->wrapperdata, response_header); } - php_stream_notify_progress_init(context, 0, file_size); + php_stream_notify_progress_init(context, 0, header_info.file_size); /* Restore original chunk size now that we're done with headers */ if (options & STREAM_WILL_CAST) @@ -991,8 +1126,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, /* restore mode */ strlcpy(stream->mode, mode, sizeof(stream->mode)); - if (transfer_encoding) { - php_stream_filter_append(&stream->readfilters, transfer_encoding); + if (header_info.transfer_encoding) { + php_stream_filter_append(&stream->readfilters, header_info.transfer_encoding); } /* It's possible that the server already sent in more data than just the headers. diff --git a/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-001.phpt b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-001.phpt new file mode 100644 index 0000000000000..f935b5a02ca94 --- /dev/null +++ b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-001.phpt @@ -0,0 +1,49 @@ +--TEST-- +GHSA-v8xr-gpvj-cx9g: Header parser of http stream wrapper does not handle folded headers (single) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html;\r\n charset=utf-8\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; charset=utf-8 +string(4) "body" +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(38) "Content-Type: text/html; charset=utf-8" +} diff --git a/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-002.phpt b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-002.phpt new file mode 100644 index 0000000000000..078d605b6718f --- /dev/null +++ b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-002.phpt @@ -0,0 +1,51 @@ +--TEST-- +GHSA-v8xr-gpvj-cx9g: Header parser of http stream wrapper does not handle folded headers (multiple) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html;\r\nCustom-Header: somevalue;\r\n param1=value1; \r\n param2=value2\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; +string(4) "body" +array(3) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(24) "Content-Type: text/html;" + [2]=> + string(54) "Custom-Header: somevalue; param1=value1; param2=value2" +} diff --git a/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-003.phpt b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-003.phpt new file mode 100644 index 0000000000000..ad5ddc879cead --- /dev/null +++ b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-003.phpt @@ -0,0 +1,49 @@ +--TEST-- +GHSA-v8xr-gpvj-cx9g: Header parser of http stream wrapper does not handle folded headers (empty) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html;\r\n \r\n charset=utf-8\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; charset=utf-8 +string(4) "body" +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(38) "Content-Type: text/html; charset=utf-8" +} diff --git a/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-004.phpt b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-004.phpt new file mode 100644 index 0000000000000..d0396e819fbd3 --- /dev/null +++ b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-004.phpt @@ -0,0 +1,48 @@ +--TEST-- +GHSA-v8xr-gpvj-cx9g: Header parser of http stream wrapper does not handle folded headers (first line) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\n Content-Type: text/html;\r\n \r\n charset=utf-8\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(file_get_contents("http://{{ ADDR }}", false, $ctx)); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: HTTP invalid response format (folding header at the start)! in %s +bool(false) +array(1) { + [0]=> + string(15) "HTTP/1.0 200 Ok" +} diff --git a/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-005.phpt b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-005.phpt new file mode 100644 index 0000000000000..037d2002cc537 --- /dev/null +++ b/ext/standard/tests/http/ghsa-v8xr-gpvj-cx9g-005.phpt @@ -0,0 +1,48 @@ +--TEST-- +GHSA-v8xr-gpvj-cx9g: Header parser of http stream wrapper does not handle folded headers (CR before header name) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\n\rIgnored: ignored\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(file_get_contents("http://{{ ADDR }}", false, $ctx)); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: HTTP invalid header name (cannot start with CR character)! in %s +bool(false) +array(1) { + [0]=> + string(15) "HTTP/1.0 200 Ok" +} diff --git a/ext/standard/tests/http/http_response_header_05.phpt b/ext/standard/tests/http/http_response_header_05.phpt deleted file mode 100644 index c5fe60fa612b7..0000000000000 --- a/ext/standard/tests/http/http_response_header_05.phpt +++ /dev/null @@ -1,30 +0,0 @@ ---TEST-- -$http_reponse_header (whitespace-only "header") ---SKIPIF-- - ---INI-- -allow_url_fopen=1 ---FILE-- - $pid, 'uri' => $uri] = http_server($responses, $output); - -$f = file_get_contents($uri); -var_dump($f); -var_dump($http_response_header); - -http_server_kill($pid); - ---EXPECT-- -string(4) "Body" -array(2) { - [0]=> - string(15) "HTTP/1.0 200 Ok" - [1]=> - string(0) "" -} From 455161cdae6ecabf7eec9f2c95ce62c30f57a6a2 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sun, 19 Jan 2025 17:49:53 +0100 Subject: [PATCH 50/58] Fix GHSA-pcmh-g36c-qc44: http headers without colon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header line must contain colon otherwise it is invalid and it needs to fail. Reviewed-by: Tim Düsterhus --- ext/standard/http_fopen_wrapper.c | 51 ++++++++++++++----- ext/standard/tests/http/bug47021.phpt | 22 ++++---- ext/standard/tests/http/bug75535.phpt | 4 +- .../tests/http/ghsa-pcmh-g36c-qc44-001.phpt | 51 +++++++++++++++++++ .../tests/http/ghsa-pcmh-g36c-qc44-002.phpt | 51 +++++++++++++++++++ 5 files changed, 154 insertions(+), 25 deletions(-) create mode 100644 ext/standard/tests/http/ghsa-pcmh-g36c-qc44-001.phpt create mode 100644 ext/standard/tests/http/ghsa-pcmh-g36c-qc44-002.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index a08aeedd11a29..9ebc28f7e3c62 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -118,6 +118,7 @@ static bool check_has_header(const char *headers, const char *header) { typedef struct _php_stream_http_response_header_info { php_stream_filter *transfer_encoding; size_t file_size; + bool error; bool follow_location; char location[HTTP_HEADER_BLOCK_SIZE]; } php_stream_http_response_header_info; @@ -127,6 +128,7 @@ static void php_stream_http_response_header_info_init( { header_info->transfer_encoding = NULL; header_info->file_size = 0; + header_info->error = false; header_info->follow_location = 1; header_info->location[0] = '\0'; } @@ -164,10 +166,11 @@ static bool php_stream_http_response_header_trim(char *http_header_line, /* Process folding headers of the current line and if there are none, parse last full response * header line. It returns NULL if the last header is finished, otherwise it returns updated * last header line. */ -static zend_string *php_stream_http_response_headers_parse(php_stream *stream, - php_stream_context *context, int options, zend_string *last_header_line_str, - char *header_line, size_t *header_line_length, int response_code, - zval *response_header, php_stream_http_response_header_info *header_info) +static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *wrapper, + php_stream *stream, php_stream_context *context, int options, + zend_string *last_header_line_str, char *header_line, size_t *header_line_length, + int response_code, zval *response_header, + php_stream_http_response_header_info *header_info) { char *last_header_line = ZSTR_VAL(last_header_line_str); size_t last_header_line_length = ZSTR_LEN(last_header_line_str); @@ -209,6 +212,19 @@ static zend_string *php_stream_http_response_headers_parse(php_stream *stream, /* Find header separator position. */ char *last_header_value = memchr(last_header_line, ':', last_header_line_length); if (last_header_value) { + /* Verify there is no space in header name */ + char *last_header_name = last_header_line + 1; + while (last_header_name < last_header_value) { + if (*last_header_name == ' ' || *last_header_name == '\t') { + header_info->error = true; + php_stream_wrapper_log_error(wrapper, options, + "HTTP invalid response format (space in header name)!"); + zend_string_efree(last_header_line_str); + return NULL; + } + ++last_header_name; + } + last_header_value++; /* Skip ':'. */ /* Strip leading whitespace. */ @@ -217,9 +233,12 @@ static zend_string *php_stream_http_response_headers_parse(php_stream *stream, last_header_value++; } } else { - /* There is no colon. Set the value to the end of the header line, which is effectively - * an empty string. */ - last_header_value = last_header_line_end; + /* There is no colon which means invalid response so error. */ + header_info->error = true; + php_stream_wrapper_log_error(wrapper, options, + "HTTP invalid response format (no colon in header line)!"); + zend_string_efree(last_header_line_str); + return NULL; } bool store_header = true; @@ -950,10 +969,16 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, if (last_header_line_str != NULL) { /* Parse last header line. */ - last_header_line_str = php_stream_http_response_headers_parse(stream, context, - options, last_header_line_str, http_header_line, &http_header_line_length, - response_code, response_header, &header_info); - if (last_header_line_str != NULL) { + last_header_line_str = php_stream_http_response_headers_parse(wrapper, stream, + context, options, last_header_line_str, http_header_line, + &http_header_line_length, response_code, response_header, &header_info); + if (EXPECTED(last_header_line_str == NULL)) { + if (UNEXPECTED(header_info.error)) { + php_stream_close(stream); + stream = NULL; + goto out; + } + } else { /* Folding header present so continue. */ continue; } @@ -983,8 +1008,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, /* If the stream was closed early, we still want to process the last line to keep BC. */ if (last_header_line_str != NULL) { - php_stream_http_response_headers_parse(stream, context, options, last_header_line_str, - NULL, NULL, response_code, response_header, &header_info); + php_stream_http_response_headers_parse(wrapper, stream, context, options, + last_header_line_str, NULL, NULL, response_code, response_header, &header_info); } if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) { diff --git a/ext/standard/tests/http/bug47021.phpt b/ext/standard/tests/http/bug47021.phpt index 326eceb687a52..168721f4ec1b6 100644 --- a/ext/standard/tests/http/bug47021.phpt +++ b/ext/standard/tests/http/bug47021.phpt @@ -70,23 +70,27 @@ do_test(1, true); echo "\n"; ?> ---EXPECT-- +--EXPECTF-- + Type='text/plain' Hello -Size=5 -World + +Warning: file_get_contents(http://%s:%d): Failed to open stream: HTTP invalid response format (no colon in header line)! in %s + Type='text/plain' Hello -Size=5 -World + +Warning: file_get_contents(http://%s:%d): Failed to open stream: HTTP invalid response format (no colon in header line)! in %s + Type='text/plain' Hello -Size=5 -World + +Warning: file_get_contents(http://%s:%d): Failed to open stream: HTTP invalid response format (no colon in header line)! in %s + Type='text/plain' Hello -Size=5 -World + +Warning: file_get_contents(http://%s:%d): Failed to open stream: HTTP invalid response format (no colon in header line)! in %s diff --git a/ext/standard/tests/http/bug75535.phpt b/ext/standard/tests/http/bug75535.phpt index 7b015890d2f51..94348d1a027aa 100644 --- a/ext/standard/tests/http/bug75535.phpt +++ b/ext/standard/tests/http/bug75535.phpt @@ -21,9 +21,7 @@ http_server_kill($pid); --EXPECT-- string(0) "" -array(2) { +array(1) { [0]=> string(15) "HTTP/1.0 200 Ok" - [1]=> - string(14) "Content-Length" } diff --git a/ext/standard/tests/http/ghsa-pcmh-g36c-qc44-001.phpt b/ext/standard/tests/http/ghsa-pcmh-g36c-qc44-001.phpt new file mode 100644 index 0000000000000..bb7945ce62d0e --- /dev/null +++ b/ext/standard/tests/http/ghsa-pcmh-g36c-qc44-001.phpt @@ -0,0 +1,51 @@ +--TEST-- +GHSA-pcmh-g36c-qc44: Header parser of http stream wrapper does not verify header name and colon (colon) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html\r\nWrong-Header\r\nGood-Header: test\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(file_get_contents("http://{{ ADDR }}", false, $ctx)); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: HTTP invalid response format (no colon in header line)! in %s +bool(false) +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(23) "Content-Type: text/html" +} diff --git a/ext/standard/tests/http/ghsa-pcmh-g36c-qc44-002.phpt b/ext/standard/tests/http/ghsa-pcmh-g36c-qc44-002.phpt new file mode 100644 index 0000000000000..1d0e4fa70a2c9 --- /dev/null +++ b/ext/standard/tests/http/ghsa-pcmh-g36c-qc44-002.phpt @@ -0,0 +1,51 @@ +--TEST-- +GHSA-pcmh-g36c-qc44: Header parser of http stream wrapper does not verify header name and colon (name) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html\r\nWrong-Header : test\r\nGood-Header: test\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(file_get_contents("http://{{ ADDR }}", false, $ctx)); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: HTTP invalid response format (space in header name)! in %s +bool(false) +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(23) "Content-Type: text/html" +} From ae14a0b9fe2ae2cacf04f47848adf5f863161f9d Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 4 Mar 2025 09:01:34 +0100 Subject: [PATCH 51/58] Fix GHSA-52jp-hrpf-2jff: http redirect location truncation It converts the allocation of location to be on heap instead of stack and errors if the location length is greater than 8086 bytes. --- ext/standard/http_fopen_wrapper.c | 87 ++++++++++++------- .../tests/http/ghsa-52jp-hrpf-2jff-001.phpt | 58 +++++++++++++ .../tests/http/ghsa-52jp-hrpf-2jff-002.phpt | 55 ++++++++++++ 3 files changed, 168 insertions(+), 32 deletions(-) create mode 100644 ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt create mode 100644 ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 9ebc28f7e3c62..e8a52c32fb81a 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -67,15 +67,16 @@ #include "php_fopen_wrappers.h" -#define HTTP_HEADER_BLOCK_SIZE 1024 -#define PHP_URL_REDIRECT_MAX 20 -#define HTTP_HEADER_USER_AGENT 1 -#define HTTP_HEADER_HOST 2 -#define HTTP_HEADER_AUTH 4 -#define HTTP_HEADER_FROM 8 -#define HTTP_HEADER_CONTENT_LENGTH 16 -#define HTTP_HEADER_TYPE 32 -#define HTTP_HEADER_CONNECTION 64 +#define HTTP_HEADER_BLOCK_SIZE 1024 +#define HTTP_HEADER_MAX_LOCATION_SIZE 8182 /* 8192 - 10 (size of "Location: ") */ +#define PHP_URL_REDIRECT_MAX 20 +#define HTTP_HEADER_USER_AGENT 1 +#define HTTP_HEADER_HOST 2 +#define HTTP_HEADER_AUTH 4 +#define HTTP_HEADER_FROM 8 +#define HTTP_HEADER_CONTENT_LENGTH 16 +#define HTTP_HEADER_TYPE 32 +#define HTTP_HEADER_CONNECTION 64 #define HTTP_WRAPPER_HEADER_INIT 1 #define HTTP_WRAPPER_REDIRECTED 2 @@ -120,17 +121,15 @@ typedef struct _php_stream_http_response_header_info { size_t file_size; bool error; bool follow_location; - char location[HTTP_HEADER_BLOCK_SIZE]; + char *location; + size_t location_len; } php_stream_http_response_header_info; static void php_stream_http_response_header_info_init( php_stream_http_response_header_info *header_info) { - header_info->transfer_encoding = NULL; - header_info->file_size = 0; - header_info->error = false; + memset(header_info, 0, sizeof(php_stream_http_response_header_info)); header_info->follow_location = 1; - header_info->location[0] = '\0'; } /* Trim white spaces from response header line and update its length */ @@ -256,7 +255,22 @@ static zend_string *php_stream_http_response_headers_parse(php_stream_wrapper *w * RFC 7238 defines 308: http://tools.ietf.org/html/rfc7238 */ header_info->follow_location = 0; } - strlcpy(header_info->location, last_header_value, sizeof(header_info->location)); + size_t last_header_value_len = strlen(last_header_value); + if (last_header_value_len > HTTP_HEADER_MAX_LOCATION_SIZE) { + header_info->error = true; + php_stream_wrapper_log_error(wrapper, options, + "HTTP Location header size is over the limit of %d bytes", + HTTP_HEADER_MAX_LOCATION_SIZE); + zend_string_efree(last_header_line_str); + return NULL; + } + if (header_info->location_len == 0) { + header_info->location = emalloc(last_header_value_len + 1); + } else if (header_info->location_len <= last_header_value_len) { + header_info->location = erealloc(header_info->location, last_header_value_len + 1); + } + header_info->location_len = last_header_value_len; + memcpy(header_info->location, last_header_value, last_header_value_len + 1); } else if (!strncasecmp(last_header_line, "Content-Type:", sizeof("Content-Type:")-1)) { php_stream_notify_info(context, PHP_STREAM_NOTIFY_MIME_TYPE_IS, last_header_value, 0); } else if (!strncasecmp(last_header_line, "Content-Length:", sizeof("Content-Length:")-1)) { @@ -560,6 +574,8 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, } } + php_stream_http_response_header_info_init(&header_info); + if (stream == NULL) goto out; @@ -941,8 +957,6 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, } } - php_stream_http_response_header_info_init(&header_info); - /* read past HTTP headers */ while (!php_stream_eof(stream)) { size_t http_header_line_length; @@ -1012,12 +1026,12 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, last_header_line_str, NULL, NULL, response_code, response_header, &header_info); } - if (!reqok || (header_info.location[0] != '\0' && header_info.follow_location)) { + if (!reqok || (header_info.location != NULL && header_info.follow_location)) { if (!header_info.follow_location || (((options & STREAM_ONLY_GET_HEADERS) || ignore_errors) && redirect_max <= 1)) { goto out; } - if (header_info.location[0] != '\0') + if (header_info.location != NULL) php_stream_notify_info(context, PHP_STREAM_NOTIFY_REDIRECTED, header_info.location, 0); php_stream_close(stream); @@ -1028,18 +1042,17 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, header_info.transfer_encoding = NULL; } - if (header_info.location[0] != '\0') { + if (header_info.location != NULL) { - char new_path[HTTP_HEADER_BLOCK_SIZE]; - char loc_path[HTTP_HEADER_BLOCK_SIZE]; + char *new_path = NULL; - *new_path='\0'; if (strlen(header_info.location) < 8 || (strncasecmp(header_info.location, "http://", sizeof("http://")-1) && strncasecmp(header_info.location, "https://", sizeof("https://")-1) && strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) && strncasecmp(header_info.location, "ftps://", sizeof("ftps://")-1))) { + char *loc_path = NULL; if (*header_info.location != '/') { if (*(header_info.location+1) != '\0' && resource->path) { char *s = strrchr(ZSTR_VAL(resource->path), '/'); @@ -1057,31 +1070,35 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, if (resource->path && ZSTR_VAL(resource->path)[0] == '/' && ZSTR_VAL(resource->path)[1] == '\0') { - snprintf(loc_path, sizeof(loc_path) - 1, "%s%s", - ZSTR_VAL(resource->path), header_info.location); + spprintf(&loc_path, 0, "%s%s", ZSTR_VAL(resource->path), header_info.location); } else { - snprintf(loc_path, sizeof(loc_path) - 1, "%s/%s", - ZSTR_VAL(resource->path), header_info.location); + spprintf(&loc_path, 0, "%s/%s", ZSTR_VAL(resource->path), header_info.location); } } else { - snprintf(loc_path, sizeof(loc_path) - 1, "/%s", header_info.location); + spprintf(&loc_path, 0, "/%s", header_info.location); } } else { - strlcpy(loc_path, header_info.location, sizeof(loc_path)); + loc_path = header_info.location; + header_info.location = NULL; } if ((use_ssl && resource->port != 443) || (!use_ssl && resource->port != 80)) { - snprintf(new_path, sizeof(new_path) - 1, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), resource->port, loc_path); + spprintf(&new_path, 0, "%s://%s:%d%s", ZSTR_VAL(resource->scheme), + ZSTR_VAL(resource->host), resource->port, loc_path); } else { - snprintf(new_path, sizeof(new_path) - 1, "%s://%s%s", ZSTR_VAL(resource->scheme), ZSTR_VAL(resource->host), loc_path); + spprintf(&new_path, 0, "%s://%s%s", ZSTR_VAL(resource->scheme), + ZSTR_VAL(resource->host), loc_path); } + efree(loc_path); } else { - strlcpy(new_path, header_info.location, sizeof(new_path)); + new_path = header_info.location; + header_info.location = NULL; } php_url_free(resource); /* check for invalid redirection URLs */ if ((resource = php_url_parse(new_path)) == NULL) { php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); + efree(new_path); goto out; } @@ -1093,6 +1110,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, while (s < e) { \ if (iscntrl(*s)) { \ php_stream_wrapper_log_error(wrapper, options, "Invalid redirect URL! %s", new_path); \ + efree(new_path); \ goto out; \ } \ s++; \ @@ -1115,6 +1133,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, stream = php_stream_url_wrap_http_ex( wrapper, new_path, mode, options, opened_path, context, --redirect_max, new_flags, response_header STREAMS_CC); + efree(new_path); } else { php_stream_wrapper_log_error(wrapper, options, "HTTP request failed! %s", tmp_line); } @@ -1127,6 +1146,10 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, efree(http_header_line); } + if (header_info.location != NULL) { + efree(header_info.location); + } + if (resource) { php_url_free(resource); } diff --git a/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt new file mode 100644 index 0000000000000..744cff9cc72f2 --- /dev/null +++ b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-001.phpt @@ -0,0 +1,58 @@ +--TEST-- +GHSA-52jp-hrpf-2jff: HTTP stream wrapper truncate redirect location to 1024 bytes (success) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $loc = str_repeat("y", 8000); + fwrite($conn, "HTTP/1.0 301 Ok\r\nContent-Type: text/html;\r\nLocation: $loc\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + case STREAM_NOTIFY_REDIRECTED: + echo "Redirected: "; + var_dump($message); + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; +Redirected: string(8000) "%s" + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: %s +string(0) "" +array(3) { + [0]=> + string(15) "HTTP/1.0 301 Ok" + [1]=> + string(24) "Content-Type: text/html;" + [2]=> + string(8010) "Location: %s" +} diff --git a/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt new file mode 100644 index 0000000000000..bc71fd4e41167 --- /dev/null +++ b/ext/standard/tests/http/ghsa-52jp-hrpf-2jff-002.phpt @@ -0,0 +1,55 @@ +--TEST-- +GHSA-52jp-hrpf-2jff: HTTP stream wrapper truncate redirect location to 1024 bytes (over limit) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $loc = str_repeat("y", 9000); + fwrite($conn, "HTTP/1.0 301 Ok\r\nContent-Type: text/html;\r\nLocation: $loc\r\n\r\nbody\r\n"); +CODE; + +$clientCode = <<<'CODE' + function stream_notification_callback($notification_code, $severity, $message, $message_code, $bytes_transferred, $bytes_max) { + switch($notification_code) { + case STREAM_NOTIFY_MIME_TYPE_IS: + echo "Found the mime-type: ", $message, PHP_EOL; + break; + case STREAM_NOTIFY_REDIRECTED: + echo "Redirected: "; + var_dump($message); + } + } + + $ctx = stream_context_create(); + stream_context_set_params($ctx, array("notification" => "stream_notification_callback")); + var_dump(trim(file_get_contents("http://{{ ADDR }}", false, $ctx))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +Found the mime-type: text/html; + +Warning: file_get_contents(http://127.0.0.1:%d): Failed to open stream: HTTP Location header size is over the limit of 8182 bytes in %s +string(0) "" +array(2) { + [0]=> + string(15) "HTTP/1.0 301 Ok" + [1]=> + string(24) "Content-Type: text/html;" +} From fac131fa95c2713c02af2fe49cbb114eb0f96047 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Fri, 14 Feb 2025 19:17:22 +0100 Subject: [PATCH 52/58] Fix GHSA-hgf5-96fm-v528: http user header check of crlf --- ext/standard/http_fopen_wrapper.c | 2 +- .../tests/http/ghsa-hgf5-96fm-v528-001.phpt | 65 +++++++++++++++++++ .../tests/http/ghsa-hgf5-96fm-v528-002.phpt | 62 ++++++++++++++++++ .../tests/http/ghsa-hgf5-96fm-v528-003.phpt | 64 ++++++++++++++++++ 4 files changed, 192 insertions(+), 1 deletion(-) create mode 100644 ext/standard/tests/http/ghsa-hgf5-96fm-v528-001.phpt create mode 100644 ext/standard/tests/http/ghsa-hgf5-96fm-v528-002.phpt create mode 100644 ext/standard/tests/http/ghsa-hgf5-96fm-v528-003.phpt diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index e8a52c32fb81a..b4d065dd0b625 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -108,7 +108,7 @@ static inline void strip_header(char *header_bag, char *lc_header_bag, static bool check_has_header(const char *headers, const char *header) { const char *s = headers; while ((s = strstr(s, header))) { - if (s == headers || *(s-1) == '\n') { + if (s == headers || (*(s-1) == '\n' && *(s-2) == '\r')) { return 1; } s++; diff --git a/ext/standard/tests/http/ghsa-hgf5-96fm-v528-001.phpt b/ext/standard/tests/http/ghsa-hgf5-96fm-v528-001.phpt new file mode 100644 index 0000000000000..c40123560ef1e --- /dev/null +++ b/ext/standard/tests/http/ghsa-hgf5-96fm-v528-001.phpt @@ -0,0 +1,65 @@ +--TEST-- +GHSA-hgf5-96fm-v528: Stream HTTP wrapper header check might omit basic auth header (incorrect inside pos) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $result = fread($conn, 1024); + $encoded_result = base64_encode($result); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html; charset=utf-8\r\n\r\n$encoded_result\r\n"); + +CODE; + +$clientCode = <<<'CODE' + $opts = [ + "http" => [ + "method" => "GET", + "header" => "Cookie: foo=bar\nauthorization:x\r\n" + ] + ]; + $ctx = stream_context_create($opts); + var_dump(explode("\r\n", base64_decode(file_get_contents("http://user:pwd@{{ ADDR }}", false, $ctx)))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +array(7) { + [0]=> + string(14) "GET / HTTP/1.1" + [1]=> + string(33) "Authorization: Basic dXNlcjpwd2Q=" + [2]=> + string(21) "Host: 127.0.0.1:%d" + [3]=> + string(17) "Connection: close" + [4]=> + string(31) "Cookie: foo=bar +authorization:x" + [5]=> + string(0) "" + [6]=> + string(0) "" +} +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(38) "Content-Type: text/html; charset=utf-8" +} diff --git a/ext/standard/tests/http/ghsa-hgf5-96fm-v528-002.phpt b/ext/standard/tests/http/ghsa-hgf5-96fm-v528-002.phpt new file mode 100644 index 0000000000000..37a47df060a1c --- /dev/null +++ b/ext/standard/tests/http/ghsa-hgf5-96fm-v528-002.phpt @@ -0,0 +1,62 @@ +--TEST-- +GHSA-hgf5-96fm-v528: Header parser of http stream wrapper does not handle folded headers (correct start pos) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $result = fread($conn, 1024); + $encoded_result = base64_encode($result); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html; charset=utf-8\r\n\r\n$encoded_result\r\n"); + +CODE; + +$clientCode = <<<'CODE' + $opts = [ + "http" => [ + "method" => "GET", + "header" => "Authorization: Bearer x\r\n" + ] + ]; + $ctx = stream_context_create($opts); + var_dump(explode("\r\n", base64_decode(file_get_contents("http://user:pwd@{{ ADDR }}", false, $ctx)))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +array(6) { + [0]=> + string(14) "GET / HTTP/1.1" + [1]=> + string(21) "Host: 127.0.0.1:%d" + [2]=> + string(17) "Connection: close" + [3]=> + string(23) "Authorization: Bearer x" + [4]=> + string(0) "" + [5]=> + string(0) "" +} +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(38) "Content-Type: text/html; charset=utf-8" +} diff --git a/ext/standard/tests/http/ghsa-hgf5-96fm-v528-003.phpt b/ext/standard/tests/http/ghsa-hgf5-96fm-v528-003.phpt new file mode 100644 index 0000000000000..6c84679ff63bd --- /dev/null +++ b/ext/standard/tests/http/ghsa-hgf5-96fm-v528-003.phpt @@ -0,0 +1,64 @@ +--TEST-- +GHSA-hgf5-96fm-v528: Header parser of http stream wrapper does not handle folded headers (correct middle pos) +--FILE-- + [ + "tcp_nodelay" => true + ] + ]); + + $server = stream_socket_server( + "tcp://127.0.0.1:0", $errno, $errstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $ctxt); + phpt_notify_server_start($server); + + $conn = stream_socket_accept($server); + + phpt_notify(message:"server-accepted"); + + $result = fread($conn, 1024); + $encoded_result = base64_encode($result); + + fwrite($conn, "HTTP/1.0 200 Ok\r\nContent-Type: text/html; charset=utf-8\r\n\r\n$encoded_result\r\n"); + +CODE; + +$clientCode = <<<'CODE' + $opts = [ + "http" => [ + "method" => "GET", + "header" => "Cookie: x=y\r\nAuthorization: Bearer x\r\n" + ] + ]; + $ctx = stream_context_create($opts); + var_dump(explode("\r\n", base64_decode(file_get_contents("http://user:pwd@{{ ADDR }}", false, $ctx)))); + var_dump($http_response_header); +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECTF-- +array(7) { + [0]=> + string(14) "GET / HTTP/1.1" + [1]=> + string(21) "Host: 127.0.0.1:%d" + [2]=> + string(17) "Connection: close" + [3]=> + string(11) "Cookie: x=y" + [4]=> + string(23) "Authorization: Bearer x" + [5]=> + string(0) "" + [6]=> + string(0) "" +} +array(2) { + [0]=> + string(15) "HTTP/1.0 200 Ok" + [1]=> + string(38) "Content-Type: text/html; charset=utf-8" +} From 054ea51d1df2ed7059a4099fe0ab2551154e0fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Wed, 20 Nov 2024 10:47:27 +0100 Subject: [PATCH 53/58] Fix GHSA-p3x9-6h7p-cgfc: libxml streams wrong `content-type` on redirect libxml streams use wrong content-type header when requesting a redirected resource. --- ext/dom/tests/ghsa-p3x9-6h7p-cgfc_001.phpt | 60 +++++++++++++++++ ext/dom/tests/ghsa-p3x9-6h7p-cgfc_002.phpt | 60 +++++++++++++++++ ext/dom/tests/ghsa-p3x9-6h7p-cgfc_003.phpt | 60 +++++++++++++++++ ext/libxml/libxml.c | 75 +++++++++++++--------- 4 files changed, 223 insertions(+), 32 deletions(-) create mode 100644 ext/dom/tests/ghsa-p3x9-6h7p-cgfc_001.phpt create mode 100644 ext/dom/tests/ghsa-p3x9-6h7p-cgfc_002.phpt create mode 100644 ext/dom/tests/ghsa-p3x9-6h7p-cgfc_003.phpt diff --git a/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_001.phpt b/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_001.phpt new file mode 100644 index 0000000000000..47212cb34100a --- /dev/null +++ b/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_001.phpt @@ -0,0 +1,60 @@ +--TEST-- +GHSA-p3x9-6h7p-cgfc: libxml streams use wrong `content-type` header when requesting a redirected resource (Basic) +--EXTENSIONS-- +dom +--SKIPIF-- + +--FILE-- + + + + GHSA-p3x9-6h7p-cgfc + + + + + + +

GHSA-p3x9-6h7p-cgfc

+ + + EOT; + // Intentionally using non-standard casing for content-type to verify it is matched not case sensitively. + yield "data://text/plain,HTTP/1.1 200 OK\r\nconteNt-tyPe: text/html; charset=utf-8\r\n\r\n{$xml}"; +} + +['pid' => $pid, 'uri' => $uri] = http_server('genResponses', $output); +$document = new \DOMDocument(); +$document->loadHTMLFile($uri); + +$h1 = $document->getElementsByTagName('h1'); +var_dump($h1->length); +var_dump($document->saveHTML()); +http_server_kill($pid); +?> +--EXPECT-- +int(1) +string(266) " + + + GHSA-p3x9-6h7p-cgfc + + + + + + +

GHSA-p3x9-6h7p-cgfc

+ + +" diff --git a/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_002.phpt b/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_002.phpt new file mode 100644 index 0000000000000..a7eff3b9a8b76 --- /dev/null +++ b/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_002.phpt @@ -0,0 +1,60 @@ +--TEST-- +GHSA-p3x9-6h7p-cgfc: libxml streams use wrong `content-type` header when requesting a redirected resource (Missing content-type) +--EXTENSIONS-- +dom +--SKIPIF-- + +--FILE-- + + + + GHSA-p3x9-6h7p-cgfc + + + + + + +

GHSA-p3x9-6h7p-cgfc

+ + + EOT; + // Missing content-type in actual response. + yield "data://text/plain,HTTP/1.1 200 OK\r\n\r\n{$xml}"; +} + +['pid' => $pid, 'uri' => $uri] = http_server('genResponses', $output); +$document = new \DOMDocument(); +$document->loadHTMLFile($uri); + +$h1 = $document->getElementsByTagName('h1'); +var_dump($h1->length); +var_dump($document->saveHTML()); +http_server_kill($pid); +?> +--EXPECT-- +int(1) +string(266) " + + + GHSA-p3x9-6h7p-cgfc + + + + + + +

GHSA-p3x9-6h7p-cgfc

+ + +" diff --git a/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_003.phpt b/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_003.phpt new file mode 100644 index 0000000000000..178b35f3525a5 --- /dev/null +++ b/ext/dom/tests/ghsa-p3x9-6h7p-cgfc_003.phpt @@ -0,0 +1,60 @@ +--TEST-- +GHSA-p3x9-6h7p-cgfc: libxml streams use wrong `content-type` header when requesting a redirected resource (Reason with colon) +--EXTENSIONS-- +dom +--SKIPIF-- + +--FILE-- + + + + GHSA-p3x9-6h7p-cgfc + + + + + + +

GHSA-p3x9-6h7p-cgfc

+ + + EOT; + // Missing content-type in actual response. + yield "data://text/plain,HTTP/1.1 200 OK: This is fine\r\n\r\n{$xml}"; +} + +['pid' => $pid, 'uri' => $uri] = http_server('genResponses', $output); +$document = new \DOMDocument(); +$document->loadHTMLFile($uri); + +$h1 = $document->getElementsByTagName('h1'); +var_dump($h1->length); +var_dump($document->saveHTML()); +http_server_kill($pid); +?> +--EXPECT-- +int(1) +string(266) " + + + GHSA-p3x9-6h7p-cgfc + + + + + + +

GHSA-p3x9-6h7p-cgfc

+ + +" diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c index 6590f73f9edd6..18ca51e36a052 100644 --- a/ext/libxml/libxml.c +++ b/ext/libxml/libxml.c @@ -525,41 +525,52 @@ php_libxml_input_buffer_create_filename(const char *URI, xmlCharEncoding enc) if (Z_TYPE(s->wrapperdata) == IS_ARRAY) { zval *header; - ZEND_HASH_FOREACH_VAL_IND(Z_ARRVAL(s->wrapperdata), header) { + /* Scan backwards: The header array might contain the headers for multiple responses, if + * a redirect was followed. + */ + ZEND_HASH_REVERSE_FOREACH_VAL_IND(Z_ARRVAL(s->wrapperdata), header) { const char buf[] = "Content-Type:"; - if (Z_TYPE_P(header) == IS_STRING && - !zend_binary_strncasecmp(Z_STRVAL_P(header), Z_STRLEN_P(header), buf, sizeof(buf)-1, sizeof(buf)-1)) { - char needle[] = "charset="; - char *haystack = estrndup(Z_STRVAL_P(header), Z_STRLEN_P(header)); - char *encoding = php_stristr(haystack, needle, Z_STRLEN_P(header), strlen(needle)); - - if (encoding) { - char *end; - - encoding += sizeof("charset=")-1; - if (*encoding == '"') { - encoding++; - } - end = strchr(encoding, ';'); - if (end == NULL) { - end = encoding + strlen(encoding); - } - end--; /* end == encoding-1 isn't a buffer underrun */ - while (*end == ' ' || *end == '\t') { - end--; - } - if (*end == '"') { - end--; - } - if (encoding >= end) continue; - *(end+1) = '\0'; - enc = xmlParseCharEncoding(encoding); - if (enc <= XML_CHAR_ENCODING_NONE) { - enc = XML_CHAR_ENCODING_NONE; + if (Z_TYPE_P(header) == IS_STRING) { + /* If no colon is found in the header, we assume it's the HTTP status line and bail out. */ + char *colon = memchr(Z_STRVAL_P(header), ':', Z_STRLEN_P(header)); + char *space = memchr(Z_STRVAL_P(header), ' ', Z_STRLEN_P(header)); + if (colon == NULL || space < colon) { + break; + } + + if (!zend_binary_strncasecmp(Z_STRVAL_P(header), Z_STRLEN_P(header), buf, sizeof(buf)-1, sizeof(buf)-1)) { + char needle[] = "charset="; + char *haystack = estrndup(Z_STRVAL_P(header), Z_STRLEN_P(header)); + char *encoding = php_stristr(haystack, needle, Z_STRLEN_P(header), sizeof("charset=")-1); + + if (encoding) { + char *end; + + encoding += sizeof("charset=")-1; + if (*encoding == '"') { + encoding++; + } + end = strchr(encoding, ';'); + if (end == NULL) { + end = encoding + strlen(encoding); + } + end--; /* end == encoding-1 isn't a buffer underrun */ + while (*end == ' ' || *end == '\t') { + end--; + } + if (*end == '"') { + end--; + } + if (encoding >= end) continue; + *(end+1) = '\0'; + enc = xmlParseCharEncoding(encoding); + if (enc <= XML_CHAR_ENCODING_NONE) { + enc = XML_CHAR_ENCODING_NONE; + } } + efree(haystack); + break; /* found content-type */ } - efree(haystack); - break; /* found content-type */ } } ZEND_HASH_FOREACH_END(); } From 17cf7cae6438bf1ad932c10ac462a6ddfdf4b798 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Wed, 13 Nov 2024 01:41:40 +0100 Subject: [PATCH 54/58] Use-after-free for ??= due to incorrect live-range calculation Fixes GHSA-rwp7-7vc6-8477 --- Zend/tests/ghsa-rwp7-7vc6-8477_001.phpt | 26 +++++++++++++++++++++++++ Zend/tests/ghsa-rwp7-7vc6-8477_002.phpt | 24 +++++++++++++++++++++++ Zend/tests/ghsa-rwp7-7vc6-8477_003.phpt | 22 +++++++++++++++++++++ Zend/zend_opcode.c | 8 ++++++++ 4 files changed, 80 insertions(+) create mode 100644 Zend/tests/ghsa-rwp7-7vc6-8477_001.phpt create mode 100644 Zend/tests/ghsa-rwp7-7vc6-8477_002.phpt create mode 100644 Zend/tests/ghsa-rwp7-7vc6-8477_003.phpt diff --git a/Zend/tests/ghsa-rwp7-7vc6-8477_001.phpt b/Zend/tests/ghsa-rwp7-7vc6-8477_001.phpt new file mode 100644 index 0000000000000..d0e9ddcba410b --- /dev/null +++ b/Zend/tests/ghsa-rwp7-7vc6-8477_001.phpt @@ -0,0 +1,26 @@ +--TEST-- +GHSA-rwp7-7vc6-8477: Use-after-free for ??= due to incorrect live-range calculation +--FILE-- +foo()->baz ??= 1; +} catch (Exception $e) { + echo $e->getMessage(); +} + +?> +--EXPECT-- +Hello diff --git a/Zend/tests/ghsa-rwp7-7vc6-8477_002.phpt b/Zend/tests/ghsa-rwp7-7vc6-8477_002.phpt new file mode 100644 index 0000000000000..4115d9eae26fd --- /dev/null +++ b/Zend/tests/ghsa-rwp7-7vc6-8477_002.phpt @@ -0,0 +1,24 @@ +--TEST-- +GHSA-rwp7-7vc6-8477: Use-after-free for ??= due to incorrect live-range calculation +--FILE-- +foo()->prop ??= 'foo'; +} catch (Error $e) { + echo $e->getMessage(); +} + +?> +--EXPECT-- +Cannot assign string to property Foo::$prop of type int diff --git a/Zend/tests/ghsa-rwp7-7vc6-8477_003.phpt b/Zend/tests/ghsa-rwp7-7vc6-8477_003.phpt new file mode 100644 index 0000000000000..f2808afbf84de --- /dev/null +++ b/Zend/tests/ghsa-rwp7-7vc6-8477_003.phpt @@ -0,0 +1,22 @@ +--TEST-- +GHSA-rwp7-7vc6-8477: Use-after-free for ??= due to incorrect live-range calculation +--FILE-- +prop ??= 'foo'; +} catch (Error $e) { + echo $e->getMessage(); +} + +?> +--EXPECT-- +Cannot assign string to property Foo::$prop of type int diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c index 55bf81376d99d..06f411a1d3663 100644 --- a/Zend/zend_opcode.c +++ b/Zend/zend_opcode.c @@ -922,6 +922,14 @@ static void zend_calc_live_ranges( opnum--; opline--; + /* SEPARATE always redeclares its op1. For the purposes of live-ranges, + * its declaration is irrelevant. Don't terminate the current live-range + * to avoid breaking special handling of COPY_TMP. */ + if (opline->opcode == ZEND_SEPARATE) { + ZEND_ASSERT(opline->op1.var == opline->result.var); + continue; + } + if ((opline->result_type & (IS_TMP_VAR|IS_VAR)) && !is_fake_def(opline)) { uint32_t var_num = EX_VAR_TO_NUM(opline->result.var) - var_offset; /* Defs without uses can occur for two reasons: Either because the result is From ebf59022926cf71af6ba1a993ce912e99b6276e4 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 18 Dec 2024 18:44:05 +0100 Subject: [PATCH 55/58] Fix GHSA-wg4p-4hqh-c3g9 --- ext/xml/tests/toffset_bounds.phpt | 42 +++++++++++++++++++++++++++++++ ext/xml/xml.c | 12 ++++++--- 2 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 ext/xml/tests/toffset_bounds.phpt diff --git a/ext/xml/tests/toffset_bounds.phpt b/ext/xml/tests/toffset_bounds.phpt new file mode 100644 index 0000000000000..5a3fd22f86cd7 --- /dev/null +++ b/ext/xml/tests/toffset_bounds.phpt @@ -0,0 +1,42 @@ +--TEST-- +XML_OPTION_SKIP_TAGSTART bounds +--EXTENSIONS-- +xml +--FILE-- +"; +$parser = xml_parser_create(); +xml_parser_set_option($parser, XML_OPTION_SKIP_TAGSTART, 100); +$res = xml_parse_into_struct($parser,$sample,$vals,$index); +var_dump($vals); +?> +--EXPECT-- +array(3) { + [0]=> + array(3) { + ["tag"]=> + string(0) "" + ["type"]=> + string(4) "open" + ["level"]=> + int(1) + } + [1]=> + array(3) { + ["tag"]=> + string(0) "" + ["type"]=> + string(8) "complete" + ["level"]=> + int(2) + } + [2]=> + array(3) { + ["tag"]=> + string(0) "" + ["type"]=> + string(5) "close" + ["level"]=> + int(1) + } +} diff --git a/ext/xml/xml.c b/ext/xml/xml.c index 62507a5d1307f..c4c1ac31a4baf 100644 --- a/ext/xml/xml.c +++ b/ext/xml/xml.c @@ -657,9 +657,11 @@ void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Ch array_init(&tag); array_init(&atr); - _xml_add_to_info(parser, ZSTR_VAL(tag_name) + parser->toffset); + char *skipped_tag_name = SKIP_TAGSTART(ZSTR_VAL(tag_name)); - add_assoc_string(&tag, "tag", SKIP_TAGSTART(ZSTR_VAL(tag_name))); /* cast to avoid gcc-warning */ + _xml_add_to_info(parser, skipped_tag_name); + + add_assoc_string(&tag, "tag", skipped_tag_name); add_assoc_string(&tag, "type", "open"); add_assoc_long(&tag, "level", parser->level); @@ -741,12 +743,14 @@ void _xml_endElementHandler(void *userData, const XML_Char *name) add_assoc_string(zv, "type", "complete"); } } else { - _xml_add_to_info(parser, ZSTR_VAL(tag_name) + parser->toffset); + char *skipped_tag_name = SKIP_TAGSTART(ZSTR_VAL(tag_name)); + + _xml_add_to_info(parser, skipped_tag_name); zval *data = xml_get_separated_data(parser); if (EXPECTED(data)) { array_init(&tag); - add_assoc_string(&tag, "tag", SKIP_TAGSTART(ZSTR_VAL(tag_name))); /* cast to avoid gcc-warning */ + add_assoc_string(&tag, "tag", skipped_tag_name); add_assoc_string(&tag, "type", "close"); add_assoc_long(&tag, "level", parser->level); zend_hash_next_index_insert(Z_ARRVAL_P(data), &tag); From d45d9a827fead3e3c746f80b70e9a4563108d762 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Fri, 7 Mar 2025 13:47:36 +0100 Subject: [PATCH 56/58] Update NEWS with entries for security fixes --- NEWS | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e644f7c0358d4..1ded2b615f3aa 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,6 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| -?? ??? ????, PHP 8.3.18 +13 Mar 2025, PHP 8.3.18 - BCMath: . Fixed bug GH-17398 (bcmul memory leak). (SakiTakamachi) @@ -15,6 +15,8 @@ PHP NEWS `__callStatic` is allowed). (timwolla) . Fixed bug GH-17797 (zend_test_compile_string crash on invalid script path). (David Carlier) + . Fixed GHSA-rwp7-7vc6-8477 (Reference counting in php_request_shutdown + causes Use-After-Free). (CVE-2024-11235) (ilutov) - DOM: . Fixed bug GH-17847 (xinclude destroys live node). (nielsdos) @@ -34,6 +36,11 @@ PHP NEWS . Fixed bug GH-17704 (ldap_search fails when $attributes contains a non-packed array with numerical keys). (nielsdos, 7u83) +- LibXML: + . Fixed GHSA-wg4p-4hqh-c3g9 (Reocurrence of #72714). (nielsdos) + . Fixed GHSA-p3x9-6h7p-cgfc (libxml streams use wrong `content-type` header + when requesting a redirected resource). (CVE-2025-1219) (timwolla) + - MBString: . Fixed bug GH-17503 (Undefined float conversion in mb_convert_variables). (cmb) @@ -69,6 +76,14 @@ PHP NEWS - Streams: . Fixed bug GH-17650 (realloc with size 0 in user_filters.c). (nielsdos) . Fix memory leak on overflow in _php_stream_scandir(). (nielsdos) + . Fixed GHSA-hgf54-96fm-v528 (Stream HTTP wrapper header check might omit + basic auth header). (CVE-2025-1736) (Jakub Zelenka) + . Fixed GHSA-52jp-hrpf-2jff (Stream HTTP wrapper truncate redirect location + to 1024 bytes). (CVE-2025-1861) (Jakub Zelenka) + . Fixed GHSA-pcmh-g36c-qc44 (Streams HTTP wrapper does not fail for headers + without colon). (CVE-2025-1734) (Jakub Zelenka) + . Fixed GHSA-v8xr-gpvj-cx9g (Header parser of `http` stream wrapper does not + handle folded headers). (CVE-2025-1217) (Jakub Zelenka) - Windows: . Fixed phpize for Windows 11 (24H2). (bwoebi) From c5e50bd327c81143558b9157930067d013dcc74b Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 11 Mar 2025 12:24:05 -0700 Subject: [PATCH 57/58] Fix NEWS versions for posterity --- NEWS | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 1ded2b615f3aa..07c3abca2c771 100644 --- a/NEWS +++ b/NEWS @@ -172,7 +172,7 @@ PHP NEWS . Fixed bug GH-17139 (Fix zip_entry_name() crash on invalid entry). (nielsdos) -02 Jan 2025, PHP 8.3.16RC1 +02 Jan 2025, PHP 8.3.16 - Core: . Fixed bug GH-17106 (ZEND_MATCH_ERROR misoptimization). (ilutov) @@ -371,7 +371,7 @@ PHP NEWS - Windows: . Fixed bug GH-16849 (Error dialog causes process to hang). (cmb) -07 Nov 2024, PHP 8.3.14RC1 +07 Nov 2024, PHP 8.3.14 - CLI: . Fixed bug GH-16373 (Shebang is not skipped for router script in cli-server @@ -1124,7 +1124,7 @@ PHP NEWS - Treewide: . Fix gcc-14 Wcalloc-transposed-args warnings. (Cristian Rodríguez) -28 Mar 2024, PHP 8.3.5RC1 +28 Mar 2024, PHP 8.3.5 - Core: . Fixed GH-13569 (GC buffer unnecessarily grows up to GC_MAX_BUF_SIZE when @@ -1393,7 +1393,7 @@ PHP NEWS . Fixed bug GH-12980 (tidynode.props.attribute is missing "Boolean Attributes" and empty attributes). (nielsdos) -07 Dec 2023, PHP 8.3.1RC1 +07 Dec 2023, PHP 8.3.1 - Core: . Fixed bug GH-12758 / GH-12768 (Invalid opline in OOM handlers within From 58c65197a22fcf2c37c72dce22f93820927c9a90 Mon Sep 17 00:00:00 2001 From: Eric Mann Date: Tue, 11 Mar 2025 13:11:02 -0700 Subject: [PATCH 58/58] Update versions for PHP 8.3.18 --- Zend/zend.h | 2 +- configure.ac | 2 +- main/php_version.h | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Zend/zend.h b/Zend/zend.h index a3e833da7ef0b..b5bed8b61187c 100644 --- a/Zend/zend.h +++ b/Zend/zend.h @@ -20,7 +20,7 @@ #ifndef ZEND_H #define ZEND_H -#define ZEND_VERSION "4.3.18-dev" +#define ZEND_VERSION "4.3.18" #define ZEND_ENGINE_3 diff --git a/configure.ac b/configure.ac index af4a1b3fc64dd..d01440cb190a9 100644 --- a/configure.ac +++ b/configure.ac @@ -17,7 +17,7 @@ dnl Basic autoconf initialization, generation of config.nice. dnl ---------------------------------------------------------------------------- AC_PREREQ([2.68]) -AC_INIT([PHP],[8.3.18-dev],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) +AC_INIT([PHP],[8.3.18],[https://github.com/php/php-src/issues],[php],[https://www.php.net]) AC_CONFIG_SRCDIR([main/php_version.h]) AC_CONFIG_AUX_DIR([build]) AC_PRESERVE_HELP_ORDER diff --git a/main/php_version.h b/main/php_version.h index 32d4dae3d7b21..d6d6d36f9d871 100644 --- a/main/php_version.h +++ b/main/php_version.h @@ -3,6 +3,6 @@ #define PHP_MAJOR_VERSION 8 #define PHP_MINOR_VERSION 3 #define PHP_RELEASE_VERSION 18 -#define PHP_EXTRA_VERSION "-dev" -#define PHP_VERSION "8.3.18-dev" +#define PHP_EXTRA_VERSION "" +#define PHP_VERSION "8.3.18" #define PHP_VERSION_ID 80318