Disabled output handler is flushed again

Description

The following code:

<?php
function test($vars) {
foreach ($vars as $i => $v1) {
foreach ($vars as $j => $v2) {
try {
$randomFunction = 'ob_flush';
$reflection = new ReflectionFunction($randomFunction);
$args = [];
$result[$randomFunction][] = $reflection->invokeArgs($args);
} catch (\Throwable $e) {
}
}
}
}
function var_fusion($var1, $var2, $var3) {
$vars = [$var1, $var2, $var3];
try{
test($vars);
} catch (ReflectionException $e) {
}
}
ob_start(function () {
throw new \Exception('ob_start');
});
var_fusion($x, $y, $z);

Resulted in this output:

/php-src/Zend/zend_string.h:200:24: runtime error: null pointer passed as argument 2, which is declared to never be null
/usr/include/string.h:44:28: note: nonnull attribute specified here
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /php-src/Zend/zend_string.h:200:24

PHP Version

PHP 8.4.0-dev

Operating System

ubuntu 22.04

Simplified reproducer:

<?php
ob_start(function () {
    throw new Exception('ob_start');
});
try {
    ob_flush();
} catch (Throwable) {}
ob_flush();

The output handler is supposed to get disabled on failure, and it does, by setting PHP_OUTPUT_HANDLER_DISABLED. So why is it still being called then...

Because this flag is checked neither in php_output_flush() nor in php_output_handler_op(). There is a respective guard in php_output_op() before calling php_output_handler_op(), but maybe that check should better be done in php_output_handler_op() itself.

guard in php_output_flush()

 main/output.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/main/output.c b/main/output.c
index c6ac741cab..369230ea7e 100644
--- a/main/output.c
+++ b/main/output.c
@@ -253,7 +253,7 @@ PHPAPI zend_result php_output_flush(void)
 {
 	php_output_context context;
 
-	if (OG(active) && (OG(active)->flags & PHP_OUTPUT_HANDLER_FLUSHABLE)) {
+	if (OG(active) && (OG(active)->flags & PHP_OUTPUT_HANDLER_FLUSHABLE) && !(OG(active)->flags & PHP_OUTPUT_HANDLER_DISABLED)) {
 		php_output_context_init(&context, PHP_OUTPUT_HANDLER_FLUSH);
 		php_output_handler_op(OG(active), &context);
 		if (context.out.data && context.out.used) {

outputs:


Notice: ob_flush(): Failed to flush buffer of Closure::__invoke (0) in C:\php-sdk\phpdev\vs17\x64\gh15181.php on line 8

guard in php_output_handler_op()

 main/output.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/main/output.c b/main/output.c
index c6ac741cab..4b6f1f0035 100644
--- a/main/output.c
+++ b/main/output.c
@@ -925,6 +925,10 @@ static inline php_output_handler_status_t php_output_handler_op(php_output_handl
 	);
 #endif
 
+	if (!(handler->flags & PHP_OUTPUT_HANDLER_DISABLED)) {
+		return PHP_OUTPUT_HANDLER_FAILURE;
+	}
+
 	if (php_output_lock_error(context->op)) {
 		/* fatal error */
 		return PHP_OUTPUT_HANDLER_FAILURE;

outputs

PS: the guard needs to be negated, though.

Right.
Since we want to guard the user call and that php_output_handler_op already can return PHP_OUTPUT_HANDLER_FAILURE, I think the most logical place to put the check is in php_output_handler_op.

I'll make a PR.

Fix phpGH-15181: Disabled output handler is flushed again

Navigation Menu

Repository navigation

Disabled output handler is flushed again #15181

Description

PHP Version

Operating System

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Disabled output handler is flushed again #15181

Description

Description

PHP Version

Operating System

Activity

ndossche commented on Jul 31, 2024

ndossche commented on Jul 31, 2024

cmb69 commented on Jul 31, 2024

ndossche commented on Jul 31, 2024

cmb69 commented on Jul 31, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions