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 1/2] 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 1a81c43a8a0..45a8cbd96de 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 00000000000..64331269dcd --- /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 cfa8eefb474..98b2fd6fe6c 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 1083564f765..09c44fec2e3 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 743662fd036..1f0e082cae8 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 34d8befe8dcef3dd665be34da5d0cf517a7d37a2 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 9 Feb 2025 14:31:28 +0100 Subject: [PATCH 2/2] Fix GH-17747: Exception on reading property in register-based FETCH_OBJ_R breaks JIT When read_property fails, it may return `&EG(uninitialized_zval)`, and the exception is handled in the VM. The VM will try to `zval_ptr_dtor_nogc` the result, but the result was never set, resulting in dtor'ing garbage data. To solve this, we check when a different zval* was returned and initialize the result with UNDEF. We don't need to copy as the slow_ex handler return values are used directly in a register. Closes GH-17749. --- NEWS | 2 ++ ext/opcache/jit/zend_jit_ir.c | 6 +++++- ext/opcache/tests/jit/gh17747.phpt | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 ext/opcache/tests/jit/gh17747.phpt diff --git a/NEWS b/NEWS index 25dbdb92de1..bc53b9f1171 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-17747 (Exception on reading property in register-based + FETCH_OBJ_R breaks JIT). (Dmitry, nielsdos) - PHPDBG: . Partially fixed bug GH-17387 (Trivial crash in phpdbg lexer). (nielsdos) diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index dd75c1f37f6..110d84d3a34 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -14539,7 +14539,11 @@ result_fetched: } if (may_throw) { - zend_jit_check_exception(jit); + if (Z_MODE(res_addr) == IS_REG) { + zend_jit_check_exception_undef_result(jit, opline); + } else { + zend_jit_check_exception(jit); + } } return 1; diff --git a/ext/opcache/tests/jit/gh17747.phpt b/ext/opcache/tests/jit/gh17747.phpt new file mode 100644 index 00000000000..803d2201ec5 --- /dev/null +++ b/ext/opcache/tests/jit/gh17747.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-17747 (Exception on reading property in register-based FETCH_OBJ_R breaks JIT) +--EXTENSIONS-- +opcache +--INI-- +opcache.jit=function +--FILE-- +a); + } +} +$test = new C; +$test->test(); +?> +--EXPECTF-- +Fatal error: Uncaught Error: Typed property C::$a must not be accessed before initialization in %s:%d +Stack trace: +#0 %s(%d): C->test() +#1 {main} + thrown in %s on line %d