From 37396ab048e2eab07afc3bfec5fedd969abe6d66 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Mon, 14 Oct 2024 16:11:37 +0200 Subject: [PATCH 1/3] Fix GH-16414: zend_test.observer.observe_function_names may segfault Unless `zend_test.observer.enabled` is on, we must not add observer handlers, so we let the INI modify handler fail early. Proper fix suggested by @bwoebi. --- ext/zend_test/observer.c | 3 +++ ext/zend_test/tests/gh16414.phpt | 11 +++++++++++ 2 files changed, 14 insertions(+) create mode 100644 ext/zend_test/tests/gh16414.phpt diff --git a/ext/zend_test/observer.c b/ext/zend_test/observer.c index d413450bf9dec..9e2ded9975dde 100644 --- a/ext/zend_test/observer.c +++ b/ext/zend_test/observer.c @@ -304,6 +304,9 @@ static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList) zend_array **p = (zend_array **) ZEND_INI_GET_ADDR(); zend_string *funcname; zend_function *func; + if (!ZT_G(observer_enabled)) { + return FAILURE; + } if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) { ZEND_HASH_FOREACH_STR_KEY(*p, funcname) { if ((func = zend_hash_find_ptr(EG(function_table), funcname))) { diff --git a/ext/zend_test/tests/gh16414.phpt b/ext/zend_test/tests/gh16414.phpt new file mode 100644 index 0000000000000..202254a914a22 --- /dev/null +++ b/ext/zend_test/tests/gh16414.phpt @@ -0,0 +1,11 @@ +--TEST-- +GH-16414 (zend_test.observer.observe_function_names may segfault) +--EXTENSIONS-- +zend_test +--FILE-- + +--EXPECT-- +bool(false) From 3d4f8e924789e56cc50720bc7c67e2cbe429e33a Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Tue, 15 Oct 2024 01:39:27 +0200 Subject: [PATCH 2/3] Must not have zend_test.observer.enabled --- ext/zend_test/tests/gh16414.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ext/zend_test/tests/gh16414.phpt b/ext/zend_test/tests/gh16414.phpt index 202254a914a22..ee1f406ebc08e 100644 --- a/ext/zend_test/tests/gh16414.phpt +++ b/ext/zend_test/tests/gh16414.phpt @@ -2,6 +2,8 @@ GH-16414 (zend_test.observer.observe_function_names may segfault) --EXTENSIONS-- zend_test +--INI-- +zend_test.observer.enabled=0 --FILE-- Date: Wed, 16 Oct 2024 19:43:49 +0200 Subject: [PATCH 3/3] Ensure that the functions have already been called We also need to ensure that the functions to observe have already been called, so that their begin and end handlers are properly initialized. Otherwise we will not observe the function execution, but a segfault. --- ext/zend_test/observer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/zend_test/observer.c b/ext/zend_test/observer.c index 9e2ded9975dde..ff3b444b610f4 100644 --- a/ext/zend_test/observer.c +++ b/ext/zend_test/observer.c @@ -309,7 +309,7 @@ static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList) } if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) { ZEND_HASH_FOREACH_STR_KEY(*p, funcname) { - if ((func = zend_hash_find_ptr(EG(function_table), funcname))) { + if ((func = zend_hash_find_ptr(EG(function_table), funcname)) && ZEND_OBSERVER_DATA(func) != NULL) { void *old_handler; zend_observer_remove_begin_handler(func, observer_begin, (zend_observer_fcall_begin_handler *)&old_handler); zend_observer_remove_end_handler(func, observer_end, (zend_observer_fcall_end_handler *)&old_handler); @@ -332,7 +332,7 @@ static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList) zend_string_release(str); if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) { ZEND_HASH_FOREACH_STR_KEY(*p, funcname) { - if ((func = zend_hash_find_ptr(EG(function_table), funcname))) { + if ((func = zend_hash_find_ptr(EG(function_table), funcname)) && ZEND_OBSERVER_DATA(func) != NULL) { zend_observer_add_begin_handler(func, observer_begin); zend_observer_add_end_handler(func, observer_end); }