From 867ed156f754af5ea4e269d6695bce03d1128ce9 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Thu, 13 Mar 2025 19:28:27 +0100
Subject: [PATCH] Fix GH-18033: NULL-ptr dereference when using
 register_tick_function in destructor

The problem is that `php_request_shutdown` calls `php_deactivate_ticks` prior
to running destructors and the shutdown functions and finalizing output
handlers.
So if a destructor or shutdown function re-registers a tick function,
then the user tick functions handler will be added back to `PG(tick_functions)`.
When the next request happens, the list `PG(tick_functions)` still contains an
entry to call the user tick functions (added in the previous request
during shutdown). This causes a NULL deref eventually because
`run_user_tick_functions` assumes that if it is called then
`BG(user_tick_functions)` must be non-NULL.

Fix this by moving the tick handler deactivation.

Closes GH-18047.
---
 NEWS                              |  2 ++
 UPGRADING                         |  3 +++
 Zend/tests/declare/gh18033_1.phpt | 24 ++++++++++++++++++++++++
 Zend/tests/declare/gh18033_2.phpt | 16 ++++++++++++++++
 main/main.c                       |  4 ++--
 5 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 Zend/tests/declare/gh18033_1.phpt
 create mode 100644 Zend/tests/declare/gh18033_2.phpt

diff --git a/NEWS b/NEWS
index bd7c6b3a26a..f6b9171d3b5 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,8 @@ PHP                                                                        NEWS
     zend.exception_string_param_max_len=0. (timwolla)
   . Fixed bug GH-17959 (Relax missing trait fatal error to error exception).
     (ilutov)
+  . Fixed bug GH-18033 (NULL-ptr dereference when using register_tick_function
+    in destructor). (nielsdos)
 
 - Curl:
   . Added curl_multi_get_handles(). (timwolla)
diff --git a/UPGRADING b/UPGRADING
index 5d372c72e21..11cca55bd17 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -38,6 +38,9 @@ PHP 8.5 UPGRADE NOTES
     resources that were indirectly collected through cycles.
   . It is now allowed to substitute static with self or the concrete class name
     in final subclasses.
+  . The tick handlers are now deactivated after all shutdown functions, destructors
+    have run and the output handlers have been cleaned up.
+    This is a consequence of fixing GH-18033.
 
 - Intl:
   . The extension now requires at least ICU 57.1.
diff --git a/Zend/tests/declare/gh18033_1.phpt b/Zend/tests/declare/gh18033_1.phpt
new file mode 100644
index 00000000000..ccce9360b46
--- /dev/null
+++ b/Zend/tests/declare/gh18033_1.phpt
@@ -0,0 +1,24 @@
+--TEST--
+GH-18033 (NULL-ptr dereference when using register_tick_function in destructor)
+--DESCRIPTION--
+Needs --repeat 2 or something similar to reproduce
+--CREDITS--
+clesmian
+--FILE--
+<?php
+class Foo {
+  function __destruct() {
+    declare(ticks=1);
+    register_tick_function(
+       function() { }
+    );
+    echo "In destructor\n";
+  }
+}
+
+$bar = new Foo;
+echo "Done\n";
+?>
+--EXPECT--
+Done
+In destructor
diff --git a/Zend/tests/declare/gh18033_2.phpt b/Zend/tests/declare/gh18033_2.phpt
new file mode 100644
index 00000000000..8fdcff1b51e
--- /dev/null
+++ b/Zend/tests/declare/gh18033_2.phpt
@@ -0,0 +1,16 @@
+--TEST--
+GH-18033 (NULL-ptr dereference when using register_tick_function in ob_start)
+--DESCRIPTION--
+Needs --repeat 2 or something similar to reproduce
+--CREDITS--
+clesmian
+--FILE--
+<?php
+ob_start(function() {
+    declare(ticks=1);
+    register_tick_function(
+       function() { }
+    );
+});
+?>
+--EXPECT--
diff --git a/main/main.c b/main/main.c
index 4075be8b377..415cb02185e 100644
--- a/main/main.c
+++ b/main/main.c
@@ -1904,8 +1904,6 @@ void php_request_shutdown(void *dummy)
 	 */
 	EG(current_execute_data) = NULL;
 
-	php_deactivate_ticks();
-
 	/* 0. Call any open observer end handlers that are still open after a zend_bailout */
 	if (ZEND_OBSERVER_ENABLED) {
 		zend_observer_fcall_end_all();
@@ -1926,6 +1924,8 @@ void php_request_shutdown(void *dummy)
 		php_output_end_all();
 	} zend_end_try();
 
+	php_deactivate_ticks();
+
 	/* 4. Reset max_execution_time (no longer executing php code after response sent) */
 	zend_try {
 		zend_unset_timeout();