From d8862f9cdb99f3a5a0f2ead6a273d85872035d8f Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 16 May 2025 20:54:56 +0100 Subject: [PATCH 1/5] GH-18572: infinite stack recursion in fallback object comparison. With nested objects and recursive comparisons, it is for now unavoidable to have a stack overflow we do some early damage control attempt early on with zend.max_allowed_stack_size check but ultimately more a band-aid than a definitive solution. --- Zend/zend_object_handlers.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index d688e4b63ed69..295bea9b803f7 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -42,6 +42,15 @@ #define IN_UNSET ZEND_GUARD_PROPERTY_UNSET #define IN_ISSET ZEND_GUARD_PROPERTY_ISSET +static zend_always_inline bool zend_objects_check_stack_limit(void) +{ +#ifdef ZEND_CHECK_STACK_LIMIT + return zend_call_stack_overflowed(EG(stack_limit)); +#else + return false; +#endif +} + /* __X accessors explanation: @@ -1714,6 +1723,11 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */ { zend_object *zobj1, *zobj2; + if (zend_objects_check_stack_limit()) { + zend_throw_error(NULL, "Object compare - stack limit reached"); + return ZEND_UNCOMPARABLE; + } + if (Z_TYPE_P(o1) != Z_TYPE_P(o2)) { /* Object and non-object */ zval *object; From 2c540abdb3cf4067784419b05cdfb4cba9324fd0 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 16 May 2025 22:28:06 +0100 Subject: [PATCH 2/5] adding test --- Zend/tests/gh18519.phpt | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 Zend/tests/gh18519.phpt diff --git a/Zend/tests/gh18519.phpt b/Zend/tests/gh18519.phpt new file mode 100644 index 0000000000000..3acec5a152908 --- /dev/null +++ b/Zend/tests/gh18519.phpt @@ -0,0 +1,34 @@ +--TEST-- +GH-18519: Nested object comparison leading to stack overflow +--FILE-- +previous = $first; +$first->next = $first; + +$cur = $first; + +for ($i = 0; $i < 50000; $i++) { + $new = new Node(); + $new->previous = $cur; + $cur->next = $new; + $new->next = $first; + $first->previous = $new; + $cur = $new; +} + +try { + // Force comparison manually to trigger zend_hash_compare + $first == $cur; +} catch(Error $e) { + echo $e->getMessage(). PHP_EOL; +} +?> +--EXPECT-- +Object compare - stack limit reached From 6fda7792b5895f791a297986040e964de2e08407 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 16 May 2025 22:33:16 +0100 Subject: [PATCH 3/5] spare the spurious warnings --- Zend/tests/gh18519.phpt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Zend/tests/gh18519.phpt b/Zend/tests/gh18519.phpt index 3acec5a152908..5a6b42a7e6eb2 100644 --- a/Zend/tests/gh18519.phpt +++ b/Zend/tests/gh18519.phpt @@ -3,6 +3,8 @@ GH-18519: Nested object comparison leading to stack overflow --FILE-- Date: Fri, 16 May 2025 22:58:25 +0100 Subject: [PATCH 4/5] will still crash under UBSAN and the check might be not triggered --- Zend/tests/gh18519.phpt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Zend/tests/gh18519.phpt b/Zend/tests/gh18519.phpt index 5a6b42a7e6eb2..1772c6079f2da 100644 --- a/Zend/tests/gh18519.phpt +++ b/Zend/tests/gh18519.phpt @@ -1,5 +1,9 @@ --TEST-- GH-18519: Nested object comparison leading to stack overflow +--SKIPIF-- + --FILE-- getMessage(). PHP_EOL; } ?> ---EXPECT-- -Object compare - stack limit reached +--EXPECTREGEX-- +(Object compare - stack limit reached|Fatal error: Nesting level too deep - recursive dependency?.+) From e13067a946785c8131034d5dacd766f1581b0ba8 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 17 May 2025 10:20:23 +0100 Subject: [PATCH 5/5] changes from feedback --- Zend/tests/gh18519.phpt | 5 ++--- Zend/zend_object_handlers.c | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Zend/tests/gh18519.phpt b/Zend/tests/gh18519.phpt index 1772c6079f2da..24881401b0615 100644 --- a/Zend/tests/gh18519.phpt +++ b/Zend/tests/gh18519.phpt @@ -7,8 +7,7 @@ if (getenv('SKIP_ASAN')) die('skip as it fatally crash'); --FILE-- --EXPECTREGEX-- -(Object compare - stack limit reached|Fatal error: Nesting level too deep - recursive dependency?.+) +(Maximum call stack size reached during object comparison|Fatal error: Nesting level too deep - recursive dependency?.+) diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 295bea9b803f7..180364b248d71 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -1724,7 +1724,7 @@ ZEND_API int zend_std_compare_objects(zval *o1, zval *o2) /* {{{ */ zend_object *zobj1, *zobj2; if (zend_objects_check_stack_limit()) { - zend_throw_error(NULL, "Object compare - stack limit reached"); + zend_throw_error(NULL, "Maximum call stack size reached during object comparison"); return ZEND_UNCOMPARABLE; }