Skip to content

Commit 12b0f1b

Browse files
bwoebibwoebi
committed
Fix #81435 Observer current_observed_frame may point to an old (overwritten) frame
Ensure current_observed_frame always points to an actually observed frame. This solution has a caveat of being O(stack size), with the worst case occurring if there are a lot of frames between the current and previous observed frames. An O(1) solution would require keeping track of the previous observed frame, which would require some additional frame attached metadata, which is best not attempted in an already released version.
1 parent 12e79dd commit 12b0f1b

File tree

5 files changed

+84
-7
lines changed

5 files changed

+84
-7
lines changed

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? 2021, PHP 8.0.12
44

5+
- Core:
6+
. Fixed bug #81435 (Observer current_observed_frame may point to an old
7+
(overwritten) frame). (Bob)
8+
59
- DOM:
610
. Fixed bug #81433 (DOMElement::setIdAttribute() called twice may remove ID).
711
(Viktor Volkov)

‎Zend/zend_observer.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,10 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
221221
current_observed_frame = NULL;
222222
} else {
223223
zend_execute_data *ex = execute_data->prev_execute_data;
224-
while (ex && !ex->func) {
224+
while (ex && (!ex->func || ex->func->type == ZEND_INTERNAL_FUNCTION
225+
|| !ZEND_OBSERVABLE_FN(ex->func->common.fn_flags)
226+
|| !ZEND_OBSERVER_DATA(&ex->func->op_array)
227+
|| ZEND_OBSERVER_DATA(&ex->func->op_array) == ZEND_OBSERVER_NOT_OBSERVED)) {
225228
ex = ex->prev_execute_data;
226229
}
227230
current_observed_frame = ex;

‎ext/zend_test/test.c‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
3434
int observer_observe_all;
3535
int observer_observe_includes;
3636
int observer_observe_functions;
37+
zend_array *observer_observe_function_names;
3738
int observer_show_return_type;
3839
int observer_show_return_value;
3940
int observer_show_init_backtrace;
@@ -329,12 +330,33 @@ static ZEND_METHOD(ZendTestNS2_Foo, method) {
329330
ZEND_PARSE_PARAMETERS_NONE();
330331
}
331332

333+
static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList)
334+
{
335+
zend_array **p = (zend_array **) ZEND_INI_GET_ADDR();
336+
if (*p) {
337+
zend_hash_release(*p);
338+
}
339+
*p = NULL;
340+
if (new_value && ZSTR_LEN(new_value)) {
341+
*p = malloc(sizeof(HashTable));
342+
_zend_hash_init(*p, 8, ZVAL_PTR_DTOR, 1);
343+
const char *start = ZSTR_VAL(new_value), *ptr;
344+
while ((ptr = strchr(start, ','))) {
345+
zend_hash_str_add_empty_element(*p, start, ptr - start);
346+
start = ptr + 1;
347+
}
348+
zend_hash_str_add_empty_element(*p, start, ZSTR_VAL(new_value) + ZSTR_LEN(new_value) - start);
349+
}
350+
return SUCCESS;
351+
}
352+
332353
PHP_INI_BEGIN()
333354
STD_PHP_INI_BOOLEAN("zend_test.observer.enabled", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_enabled, zend_zend_test_globals, zend_test_globals)
334355
STD_PHP_INI_BOOLEAN("zend_test.observer.show_output", "1", PHP_INI_SYSTEM, OnUpdateBool, observer_show_output, zend_zend_test_globals, zend_test_globals)
335356
STD_PHP_INI_BOOLEAN("zend_test.observer.observe_all", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_observe_all, zend_zend_test_globals, zend_test_globals)
336357
STD_PHP_INI_BOOLEAN("zend_test.observer.observe_includes", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_observe_includes, zend_zend_test_globals, zend_test_globals)
337358
STD_PHP_INI_BOOLEAN("zend_test.observer.observe_functions", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_observe_functions, zend_zend_test_globals, zend_test_globals)
359+
STD_PHP_INI_ENTRY("zend_test.observer.observe_function_names", "", PHP_INI_SYSTEM, zend_test_observer_OnUpdateCommaList, observer_observe_function_names, zend_zend_test_globals, zend_test_globals)
338360
STD_PHP_INI_BOOLEAN("zend_test.observer.show_return_type", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_return_type, zend_zend_test_globals, zend_test_globals)
339361
STD_PHP_INI_BOOLEAN("zend_test.observer.show_return_value", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_return_value, zend_zend_test_globals, zend_test_globals)
340362
STD_PHP_INI_BOOLEAN("zend_test.observer.show_init_backtrace", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_init_backtrace, zend_zend_test_globals, zend_test_globals)
@@ -584,10 +606,16 @@ static zend_observer_fcall_handlers observer_fcall_init(zend_execute_data *execu
584606

585607
if (ZT_G(observer_observe_all)) {
586608
return (zend_observer_fcall_handlers){observer_begin, observer_end};
587-
} else if (ZT_G(observer_observe_includes) && !fbc->common.function_name) {
588-
return (zend_observer_fcall_handlers){observer_begin, observer_end};
589-
} else if (ZT_G(observer_observe_functions) && fbc->common.function_name) {
590-
return (zend_observer_fcall_handlers){observer_begin, observer_end};
609+
} else if (fbc->common.function_name) {
610+
if (ZT_G(observer_observe_functions)) {
611+
return (zend_observer_fcall_handlers){observer_begin, observer_end};
612+
} else if (ZT_G(observer_observe_function_names) && zend_hash_exists(ZT_G(observer_observe_function_names), fbc->common.function_name)) {
613+
return (zend_observer_fcall_handlers){observer_begin, observer_end};
614+
}
615+
} else {
616+
if (ZT_G(observer_observe_includes)) {
617+
return (zend_observer_fcall_handlers){observer_begin, observer_end};
618+
}
591619
}
592620
return (zend_observer_fcall_handlers){NULL, NULL};
593621
}

‎ext/zend_test/tests/bug81435.phpt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
--TEST--
2+
Bug #81435 (Observer current_observed_frame may point to an old (overwritten) frame)
3+
--INI--
4+
memory_limit=20M
5+
zend_test.observer.enabled=1
6+
zend_test.observer.observe_function_names=a,d
7+
--FILE--
8+
<?php
9+
10+
function d() {} // observed
11+
12+
function c() {
13+
d();
14+
}
15+
16+
function b() {
17+
c();
18+
}
19+
20+
function bailout(...$args) {
21+
array_map("str_repeat", ["\xFF"], [100000000]);
22+
}
23+
24+
function a() { // observed (first_observed_frame)
25+
b();
26+
bailout(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15); // overwrite the vm_stack containing prev_execute_data
27+
}
28+
29+
a();
30+
31+
?>
32+
--EXPECTF--
33+
<!-- init '%s' -->
34+
<!-- init a() -->
35+
<a>
36+
<!-- init b() -->
37+
<!-- init c() -->
38+
<!-- init d() -->
39+
<d>
40+
</d>
41+
<!-- init bailout() -->
42+
43+
Fatal error: Allowed memory size of 20971520 bytes exhausted at %s (tried to allocate %d bytes) in %s on line %d
44+
</a>

‎ext/zend_test/tests/observer_error_05.phpt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ Observer: End handlers fire after a userland fatal error
66
zend_test.observer.enabled=1
77
zend_test.observer.observe_all=1
88
zend_test.observer.show_return_value=1
9-
--XFAIL--
10-
This is unsafe and fails on macos
119
--FILE--
1210
<?php
1311
set_error_handler(function ($errno, $errstr, $errfile, $errline) {

0 commit comments

Comments
 (0)