Fix GH-18033: NULL-ptr dereference when using register_tick_function in destructor #18047
Conversation
…on 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.
| @@ -1926,6 +1924,8 @@ void php_request_shutdown(void *dummy) | |||
| php_output_end_all(); | |||
| } zend_end_try(); | |||
|
|
|||
| php_deactivate_ticks(); | |||
There was a problem hiding this comment.
I think the same issue can occur with buffered output handlers. Does this need to be moved down further?
There was a problem hiding this comment.
This is already below php_output_end_all and I don't think php_output_deactivate can call user code?
There was a problem hiding this comment.
Ah, you're probably right! Can't verify as I'm ok mobile.
There was a problem hiding this comment.
I think it doesn't call user code, at least not on purpose. Destructors are already called, so destroying the user output handler will not call any user code. The only way for php_output_deactivate() to execute user code would be if it invoked an error handler. But this would be unsafe after zend_deactivate_modules(), so it probably doesn't.
There was a problem hiding this comment.
Indeed. There's even a test. I just didn't look at it well enough.
| @@ -1926,6 +1924,8 @@ void php_request_shutdown(void *dummy) | |||
| php_output_end_all(); | |||
| } zend_end_try(); | |||
|
|
|||
| php_deactivate_ticks(); | |||
There was a problem hiding this comment.
I think it doesn't call user code, at least not on purpose. Destructors are already called, so destroying the user output handler will not call any user code. The only way for php_output_deactivate() to execute user code would be if it invoked an error handler. But this would be unsafe after zend_deactivate_modules(), so it probably doesn't.
The problem is that
php_request_shutdowncallsphp_deactivate_ticksprior 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 listPG(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 becauserun_user_tick_functionsassumes that if it is called thenBG(user_tick_functions)must be non-NULL.Fix this by moving the tick handler deactivation.