diff --git a/NEWS b/NEWS index df90db4bfd0..8c62fb8ebf4 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,8 @@ PHP NEWS . Fixed bug GH-20695 (Assertion failure in normalize_value() when parsing malformed INI input via parse_ini_string()). (ndossche) . Fixed bug GH-20714 (Uncatchable exception thrown in generator). (ilutov) + . Fixed bug GH-20352 (UAF in php_output_handler_free via re-entrant + ob_start() during error deactivation). (ndossche) - Bz2: . Fixed bug GH-20620 (bzcompress overflow on large source size). diff --git a/main/output.c b/main/output.c index ef6be672d1c..4e542318139 100644 --- a/main/output.c +++ b/main/output.c @@ -187,8 +187,12 @@ PHPAPI void php_output_deactivate(void) /* release all output handlers */ if (OG(handlers).elements) { while ((handler = zend_stack_top(&OG(handlers)))) { - php_output_handler_free(handler); zend_stack_del_top(&OG(handlers)); + /* It's possible to start a new output handler and mark it as active, + * however this loop will destroy all active handlers. */ + OG(active) = NULL; + ZEND_ASSERT(OG(running) == NULL && "output is deactivated therefore running should stay NULL"); + php_output_handler_free(handler); } } zend_stack_destroy(&OG(handlers)); @@ -718,10 +722,11 @@ PHPAPI void php_output_handler_dtor(php_output_handler *handler) * Destroy and free an output handler */ PHPAPI void php_output_handler_free(php_output_handler **h) { - if (*h) { - php_output_handler_dtor(*h); - efree(*h); + php_output_handler *handler = *h; + if (handler) { *h = NULL; + php_output_handler_dtor(handler); + efree(handler); } } /* }}} */ diff --git a/tests/output/gh20352.phpt b/tests/output/gh20352.phpt new file mode 100644 index 00000000000..16be0b920e8 --- /dev/null +++ b/tests/output/gh20352.phpt @@ -0,0 +1,24 @@ +--TEST-- +GH-20352 (UAF in php_output_handler_free via re-entrant ob_start() during error deactivation) +--FILE-- + +--EXPECTF-- +Fatal error: ob_start(): Cannot use output buffering in output buffering display handlers in %s on line %d