Skip to content

Stream-error finalization uses streams after close #23259

Description

@edorian

Description

Fuzzing on the new PHP 8.6 streams API, found by Ryan @ Calif.io.


The following code:

<?php

// A PHAR flush error leaves fclose() finalizing errors through a freed stream.

$directory = sys_get_temp_dir() . '/php-stream-error-phar-' . getmypid();
$archive = $directory . '/close-error.phar';

mkdir($directory);
$phar = new Phar($archive);
$phar['seed.txt'] = 'seed';
unset($phar);

$stream = fopen("phar://$archive/new.txt", 'wb');
fwrite($stream, str_repeat('A', 4096));

unlink($archive);
rmdir($directory);

var_dump(fclose($stream));


class OversizedReadStream
{
    public $context;
    private bool $read = false;

    public function stream_open($path, $mode, $options, &$openedPath): bool
    {
        return true;
    }

    public function stream_read(int $count): string
    {
        $this->read = true;
        return str_repeat('A', $count + 1);
    }

    public function stream_eof(): bool
    {
        return $this->read;
    }

    public function stream_stat(): array
    {
        return [];
    }
}

stream_wrapper_register('oversized-read', OversizedReadStream::class);
var_dump(get_meta_tags('oversized-read://input'));

Resulted in this output:

 Error: Exit code 1
 =================================================================
 ==187==ERROR: AddressSanitizer: heap-use-after-free on address 0xffff8122b560 at pc 0xaaaacd023f44 bp 0xffffd11235c0 sp 0xffffd11235d8
 READ of size 8 at 0xffff8122b560 thread T0
         #0 0xaaaacd023f40 in php_stream_error_operation_end_for_stream /opt/php-src/main/streams/stream_errors.c:554
         #1 0xaaaacc6d5dbc in zif_fclose /opt/php-src/ext/standard/file.c:782
         #2 0xaaaacd5d0068 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /opt/php-src/Zend/zend_vm_execute.h:1393
         #3 0xaaaacd8dc8d0 in execute_ex /opt/php-src/Zend/zend_vm_execute.h:110749
         #4 0xaaaacd8fa6f8 in zend_execute /opt/php-src/Zend/zend_vm_execute.h:115963
         #5 0xaaaacdc2aadc in zend_execute_script /opt/php-src/Zend/zend.c:1980

But I expected this output instead:

# STDOUT
     
bool(true)
array(0) {
}

# STDERR

PHP Warning:  fclose(): unable to seek to start of file "seed.txt" 
while creating new phar "/tmp/php-stream-error-phar-16898/close-error.phar"
    [....]

For validation, I used the followingpatch;, it's mostly generated, and I don't have a deep understanding of the news streams code, hence a bug report and not a PR.

Diffs used for testing
diff --git a/ext/standard/file.c b/ext/standard/file.c
index fd6f578e..9a908c81 100644
--- a/ext/standard/file.c
+++ b/ext/standard/file.c
@@ -367,9 +367,10 @@ PHP_FUNCTION(get_meta_tags)
 
 	if (value) efree(value);
 	if (name) efree(name);
+	const php_stream_context *context = PHP_STREAM_CONTEXT(md.stream);
 	php_stream_close(md.stream);
 
-	php_stream_error_operation_end_for_stream(md.stream);
+	php_stream_error_operation_end_for_context(context);
 }
 /* }}} */
 
@@ -776,10 +777,11 @@ PHPAPI PHP_FUNCTION(fclose)
 	}
 
 	php_stream_error_operation_begin();
+	const php_stream_context *context = PHP_STREAM_CONTEXT(stream);
 	php_stream_free(stream,
 		PHP_STREAM_FREE_KEEP_RSRC |
 		(stream->is_persistent ? PHP_STREAM_FREE_CLOSE_PERSISTENT : PHP_STREAM_FREE_CLOSE));
-	php_stream_error_operation_end_for_stream(stream);
+	php_stream_error_operation_end_for_context(context);
 
 	RETURN_TRUE;
 }
diff --git a/ext/standard/tests/streams/stream_error_finalize_after_close.phpt b/ext/standard/tests/streams/stream_error_finalize_after_close.phpt
new file mode 100644
index 00000000..e9e20399
--- /dev/null
+++ b/ext/standard/tests/streams/stream_error_finalize_after_close.phpt
@@ -0,0 +1,47 @@
+--TEST--
+GH-XXXXX (fclose/get_meta_tags): stream-error finalization must not use a stream after it is freed
+--EXTENSIONS--
+phar
+--INI--
+phar.readonly=0
+--FILE--
+<?php
+// A PHAR flush error leaves fclose() finalizing errors through a freed stream.
+$directory = sys_get_temp_dir() . '/php-stream-error-phar-' . getmypid();
+$archive = $directory . '/close-error.phar';
+
+mkdir($directory);
+$phar = new Phar($archive);
+$phar['seed.txt'] = 'seed';
+unset($phar);
+
+$stream = fopen("phar://$archive/new.txt", 'wb');
+fwrite($stream, str_repeat('A', 4096));
+
+unlink($archive);
+rmdir($directory);
+
+var_dump(fclose($stream));
+
+class OversizedReadStream
+{
+    public $context;
+    private bool $read = false;
+    public function stream_open($path, $mode, $options, &$openedPath): bool { return true; }
+    public function stream_read(int $count): string { $this->read = true; return str_repeat('A', $count + 1); }
+    public function stream_eof(): bool { return $this->read; }
+    public function stream_stat(): array { return []; }
+}
+
+stream_wrapper_register('oversized-read', OversizedReadStream::class);
+var_dump(get_meta_tags('oversized-read://input'));
+?>
+--EXPECTF--
+Warning: fclose(): unable to seek to start of file "seed.txt" while creating new phar "%s" in %s on line %d
+
+Warning: fclose(): unable to seek to start of file "seed.txt" while creating new phar "%s" in %s on line %d
+bool(true)
+
+Warning: get_meta_tags(): OversizedReadStream::stream_read - read 1 bytes more data than requested (8193 read, 8192 max) - excess data will be lost in %s on line %d
+array(0) {
+}
diff --git a/main/streams/php_stream_errors.h b/main/streams/php_stream_errors.h
index da7a4cc8..fb62201d 100644
--- a/main/streams/php_stream_errors.h
+++ b/main/streams/php_stream_errors.h
@@ -94,6 +94,7 @@ typedef struct {
 /* Error operation management */
 PHPAPI php_stream_error_operation *php_stream_error_operation_begin(void);
 PHPAPI void php_stream_error_operation_end(const php_stream_context *context);
+PHPAPI void php_stream_error_operation_end_for_context(const php_stream_context *context);
 PHPAPI void php_stream_error_operation_end_for_stream(const php_stream *stream);
 PHPAPI void php_stream_error_operation_abort(void);
 
diff --git a/main/streams/stream_errors.c b/main/streams/stream_errors.c
index 4e59e275..d617a2d7 100644
--- a/main/streams/stream_errors.c
+++ b/main/streams/stream_errors.c
@@ -533,7 +533,7 @@ PHPAPI void php_stream_error_operation_end(const php_stream_context *context)
 	op->error_count = 0;
 }
 
-PHPAPI void php_stream_error_operation_end_for_stream(const php_stream *stream)
+PHPAPI void php_stream_error_operation_end_for_context(const php_stream_context *context)
 {
 	php_stream_error_state *state = &FG(stream_error_state);
 	php_stream_error_operation *op = state->current_operation;
@@ -551,10 +551,14 @@ PHPAPI void php_stream_error_operation_end_for_stream(const php_stream *stream)
 		return;
 	}
 
-	const php_stream_context *context = PHP_STREAM_CONTEXT(stream);
 	php_stream_error_operation_end(context);
 }
 
+PHPAPI void php_stream_error_operation_end_for_stream(const php_stream *stream)
+{
+	php_stream_error_operation_end_for_context(PHP_STREAM_CONTEXT(stream));
+}
+
 PHPAPI void php_stream_error_operation_abort(void)
 {
 	php_stream_error_state *state = &FG(stream_error_state);

PHP Version

  PHP 8.6.0-dev (cli) (built: Aug 11 2026 17:26:03) (ZTS DEBUG)
  Copyright © The PHP Group and Contributors
  Zend Engine v4.6.0-dev, Copyright © Zend by Perforce
      with Zend OPcache v8.6.0-dev, Copyright ©, by Zend by Perforce

Operating System

No response

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions