Fix stream double free in phar #18953

dixyes · 2025-06-27T03:28:34Z

I've encountered a bug that simply this code can trigger:

<?php

declare(strict_types=1);

require 'bug/autoload.php';

// cleaning
@unlink("bug.phar");
@unlink("bug.phar.gz");

// create a phar
$phar = new Phar("bug.phar");
$phar->startBuffering();
// add any dir
$phar->addEmptyDir("dir");
$phar->stopBuffering();
// compress
$phar->compress(Phar::GZ);

// this increases chance when zendmm enabled, don't know why
// $obj1 = new NS1\Class1();
// $obj2 = new NS1\Class1();

With zendmm. Codes below increases reproduce chance

<?php
// bug/autoload.php
spl_autoload_register(function ($class) {
    $base_dir = __DIR__ . '/src/';
    
    $file = $base_dir . str_replace('\\', '/', $class) . '.php';
    if (file_exists($file)) {
        require $file;
    }
});

<?php
// bug/src/NS1/Class1.php

declare(strict_types=1);

namespace NS1;

use NS2\Interface1;

class Class1 implements Interface1
{
}

<?php
// bug/src/NS2/Interface1.php

declare(strict_types=1);

namespace NS2;

interface Interface1
{
}

It's a UAF/double free, but it's with very little probability because of zendmm / memory allocater.

With help of ASan, this can be triggered every time.

USE_ZEND_ALLOC=0 php this_file.php

=================================================================
==66668==ERROR: AddressSanitizer: heap-use-after-free on address 0x11fe20aa46d0 at pc 0x7ffb656b330f bp 0x007e055fdb20 sp 0x007e055fdb20
READ of size 8 at 0x11fe20aa46d0 thread T0
    #0 0x7ffb656b330e in _php_stream_free C:\path\to\php\php-src\main\streams\streams.c:384
    #1 0x7ffb65c49bd7 in destroy_phar_manifest_entry_int C:\path\to\php\php-src\ext\phar\phar.c:365
    #2 0x7ffb65c45425 in destroy_phar_manifest_entry C:\path\to\php\php-src\ext\phar\phar.c:388
    #3 0x7ffb654b7365 in zend_hash_destroy C:\path\to\php\php-src\Zend\zend_hash.c:1773
    #4 0x7ffb65c422be in phar_destroy_phar_data C:\path\to\php\php-src\ext\phar\phar.c:213
    #5 0x7ffb65c4c167 in destroy_phar_data_only C:\path\to\php\php-src\ext\phar\phar.c:295
    #6 0x7ffb65c4924a in destroy_phar_data C:\path\to\php\php-src\ext\phar\phar.c:340
    #7 0x7ffb654b7365 in zend_hash_destroy C:\path\to\php\php-src\Zend\zend_hash.c:1773
    #8 0x7ffb65c4b730 in zm_deactivate_phar C:\path\to\php\php-src\ext\phar\phar.c:3512
    #9 0x7ffb6531d71a in zend_deactivate_modules C:\path\to\php\php-src\Zend\zend_API.c:3424
    #10 0x7ffb6567728a in php_request_shutdown C:\path\to\php\php-src\main\main.c:1946
    #11 0x7ff644ee3a69 in do_cli C:\path\to\php\php-src\sapi\cli\php_cli.c:1159
    #12 0x7ff644ee2509 in main C:\path\to\php\php-src\sapi\cli\php_cli.c:1363
    #13 0x7ff644ef6e47 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #14 0x7ffcd049e8d6 in BaseThreadInitThunk+0x16 (C:\WINDOWS\System32\KERNEL32.DLL+0x18002e8d6)
    #15 0x7ffcd10fc34b in RtlUserThreadStart+0x2b (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18003c34b)

0x11fe20aa46d0 is located 144 bytes inside of 224-byte region [0x11fe20aa4640,0x11fe20aa4720)
freed by thread T0 here:
    #0 0x7ffc0e1cd408 in _asan_wrap__CrtIsValidHeapPointer+0xb78 (C:\path\to\proxyagent-ng\clang_rt.asan_dynamic-x86_64.dll+0x18004d408)
    #1 0x7ffb6533b4e9 in __zend_free C:\path\to\php\php-src\Zend\zend_alloc.c:3546
    #2 0x7ffb6533acfc in _efree C:\path\to\php\php-src\Zend\zend_alloc.c:2772
    #3 0x7ffb656b384c in _php_stream_free C:\path\to\php\php-src\main\streams\streams.c:528
    #4 0x7ffb65c463a1 in phar_flush_ex C:\path\to\php\php-src\ext\phar\phar.c:2709
    #5 0x7ffb65c71bad in phar_rename_archive C:\path\to\php\php-src\ext\phar\phar_object.c:2200
    #6 0x7ffb65c6daa0 in phar_convert_to_other C:\path\to\php\php-src\ext\phar\phar_object.c:2335
    #7 0x7ffb65c56d1e in zim_Phar_compress C:\path\to\php\php-src\ext\phar\phar_object.c:3271
    #8 0x7ffb653c929b in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER C:\path\to\php\php-src\Zend\zend_vm_execute.h:1998
    #9 0x7ffb65479e51 in execute_ex_real C:\path\to\php\php-src\Zend\zend_vm_execute.h:58704
    #10 0x7ffb65d0829c in execute_ex+0x4c (C:\path\to\php\inst\php8ts.dll+0x1809f829c)
    #11 0x7ffb653bb99f in zend_execute C:\path\to\php\php-src\Zend\zend_vm_execute.h:64393
    #12 0x7ffb65318d0b in zend_execute_script C:\path\to\php\php-src\Zend\zend.c:1943
    #13 0x7ffb656788ef in php_execute_script_ex C:\path\to\php\php-src\main\main.c:2594
    #14 0x7ffb6567843a in php_execute_script C:\path\to\php\php-src\main\main.c:2634
    #15 0x7ff644ee41b1 in do_cli C:\path\to\php\php-src\sapi\cli\php_cli.c:952
    #16 0x7ff644ee2509 in main C:\path\to\php\php-src\sapi\cli\php_cli.c:1363
    #17 0x7ff644ef6e47 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #18 0x7ffcd049e8d6 in BaseThreadInitThunk+0x16 (C:\WINDOWS\System32\KERNEL32.DLL+0x18002e8d6)
    #19 0x7ffcd10fc34b in RtlUserThreadStart+0x2b (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18003c34b)

previously allocated by thread T0 here:
    #0 0x7ffc0e1cd558 in _asan_wrap__CrtIsValidHeapPointer+0xcc8 (C:\path\to\proxyagent-ng\clang_rt.asan_dynamic-x86_64.dll+0x18004d558)
    #1 0x7ffb6533b42e in __zend_malloc C:\path\to\php\php-src\Zend\zend_alloc.c:3518
    #2 0x7ffb6533aafc in _emalloc C:\path\to\php\php-src\Zend\zend_alloc.c:2762
    #3 0x7ffb656b2c9d in _php_stream_alloc C:\path\to\php\php-src\main\streams\streams.c:284
    #4 0x7ffb656b11fc in _php_stream_fopen_from_fd_int C:\path\to\php\php-src\main\streams\plain_wrapper.c:194   
    #5 0x7ffb656ae564 in _php_stream_fopen_temporary_file C:\path\to\php\php-src\main\streams\plain_wrapper.c:230
    #6 0x7ffb656ae3f6 in _php_stream_fopen_tmpfile C:\path\to\php\php-src\main\streams\plain_wrapper.c:252       
    #7 0x7ffb65c846c3 in phar_get_or_create_entry_data C:\path\to\php\php-src\ext\phar\util.c:683
    #8 0x7ffb65c7034e in phar_mkdir C:\path\to\php\php-src\ext\phar\phar_object.c:3730
    #9 0x7ffb65c542c4 in zim_Phar_addEmptyDir C:\path\to\php\php-src\ext\phar\phar_object.c:3860
    #10 0x7ffb653c929b in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER C:\path\to\php\php-src\Zend\zend_vm_execute.h:1998
    #11 0x7ffb65479e51 in execute_ex_real C:\path\to\php\php-src\Zend\zend_vm_execute.h:58704
    #12 0x7ffb65d0829c in execute_ex+0x4c (C:\path\to\php\inst\php8ts.dll+0x1809f829c)
    #13 0x7ffb653bb99f in zend_execute C:\path\to\php\php-src\Zend\zend_vm_execute.h:64393
    #14 0x7ffb65318d0b in zend_execute_script C:\path\to\php\php-src\Zend\zend.c:1943
    #15 0x7ffb656788ef in php_execute_script_ex C:\path\to\php\php-src\main\main.c:2594
    #16 0x7ffb6567843a in php_execute_script C:\path\to\php\php-src\main\main.c:2634
    #17 0x7ff644ee41b1 in do_cli C:\path\to\php\php-src\sapi\cli\php_cli.c:952
    #18 0x7ff644ee2509 in main C:\path\to\php\php-src\sapi\cli\php_cli.c:1363
    #19 0x7ff644ef6e47 in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #20 0x7ffcd049e8d6 in BaseThreadInitThunk+0x16 (C:\WINDOWS\System32\KERNEL32.DLL+0x18002e8d6)
    #21 0x7ffcd10fc34b in RtlUserThreadStart+0x2b (C:\WINDOWS\SYSTEM32\ntdll.dll+0x18003c34b)

SUMMARY: AddressSanitizer: heap-use-after-free C:\path\to\php\php-src\main\streams\streams.c:384 in _php_stream_free
Shadow bytes around the buggy address:
  0x11fe20aa4400: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x11fe20aa4480: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
  0x11fe20aa4500: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x11fe20aa4580: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x11fe20aa4600: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x11fe20aa4680: fd fd fd fd fd fd fd fd fd fd[fd]fd fd fd fd fd
  0x11fe20aa4700: fd fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa
  0x11fe20aa4780: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x11fe20aa4800: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa
  0x11fe20aa4880: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x11fe20aa4900: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==66668==ABORTING

I used a straightforward way to fix it, but it's hard to write a test file to check this with zendmm

Girgias · 2025-06-27T10:27:02Z

ext/phar/tests/gh18953.phpt

+// cleaning
+@unlink("gh18953.phar");
+@unlink("gh18953.phar.gz");


This shouldn't be needed because of the CLEAN section

If last test failed without cleaning, this test will never success. So I unlink them here.

A CLEAN section always runs, if it doesn't there are bigger problems with the test runner. Please remove it as asked previously.

Girgias · 2025-06-27T10:27:45Z

ext/phar/tests/gh18953/autoload.inc

+spl_autoload_register(function ($class) {
+    $base_dir = __DIR__ . '/src/';
+
+    $file = $base_dir . str_replace('\\', '/', $class) . '.inc';
+    if (file_exists($file)) {
+        require $file;
+    }
+});


Can you not move this into the base test file?

Not sure require matters, so I splited them

nielsdos · 2025-06-28T08:54:27Z

ext/phar/phar_object.c

@@ -2306,6 +2306,13 @@ static zend_object *phar_convert_to_other(phar_archive_data *source, int convert
 			/* exception already thrown */
 			return NULL;
 		}
+
+		if (newentry.fp == NULL) {


This looks like the wrong solution. You're resetting the file pointer but phar_copy_file_contents already used it.
I believe the right solution is to:

Reset fp and cfp right after the copy from entry to newentry. The new file pointer will then be opened and set correctly in phar_copy_file_contents

Remove the /* save for potential restore on error */ block that moves the file pointers as this recovery mechanism isn't implemented as far as I see.

It's hard for me to understand why phar codes use cfp, so I use this strange solution to avoid break other features.

Also, there are many memcpys (like newentry <- entry here), dont know why, so I chose to not modify phar_copy_file_contents

nielsdos · 2025-06-28T08:54:34Z

ext/phar/tests/gh18953.phpt

+--INI--
+phar.readonly=0
+--ENV--
+USE_ZEND_ALLOC=0


phar.readonly=0 is needed, as for USE_ZEND_ALLOC=0, it helps reproducing in common php test environment (without ASAN)

nielsdos · 2025-06-28T08:55:02Z

ext/phar/tests/gh18953.phpt

+
+declare(strict_types=1);
+
+require __DIR__ . '/gh18953/autoload.inc';


You don't need all this autoloading stuff, nor the classes at the bottom. Executing the test without these things, with ASAN+USE_ZEND_ALLOC=0, reliably reproduces the bug.

dixyes · 2025-06-29T02:17:35Z

@Girgias @nielsdos

Ths PR is more like a bug report rather than a bug fix. It's so hard for someone like me to read the entire phar code and make real good fixes, this is a patch only managing to work for my own project. I'm eager to work on my own project.

If cfp is useless, it's beter to remove it and simplify codes, this is a task beyond my capability, but I can use this simple patch to continue my own project and I would prefer to wait for real fixes.

dixyes · 2025-06-29T02:25:50Z

I wont modify this PR because it's really not a good fix (and should not be merged/fixed in this way), and wait for better fixes.

Fix stream double free in phar

6060b5e

github-actions bot added the Extension: phar label Jun 27, 2025

dixyes marked this pull request as ready for review June 27, 2025 03:41

Add test for fix-phar-stream-double-free

5b94a74

dixyes force-pushed the fix-phar-stream-double-free branch from 517e8ad to 5b94a74 Compare June 27, 2025 03:54

devnexen requested a review from nielsdos June 27, 2025 07:13

Girgias reviewed Jun 27, 2025

View reviewed changes

nielsdos requested changes Jun 28, 2025

View reviewed changes

dixyes marked this pull request as draft June 29, 2025 02:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix stream double free in phar #18953

Fix stream double free in phar #18953

dixyes commented Jun 27, 2025

Uh oh!

Girgias Jun 27, 2025

Uh oh!

dixyes Jun 29, 2025

Uh oh!

Girgias Jun 29, 2025

Uh oh!

Girgias Jun 27, 2025

Uh oh!

dixyes Jun 29, 2025

Uh oh!

nielsdos Jun 28, 2025

Uh oh!

dixyes Jun 29, 2025

Uh oh!

nielsdos Jun 28, 2025

Uh oh!

dixyes Jun 29, 2025

Uh oh!

nielsdos Jun 28, 2025

Uh oh!

dixyes commented Jun 29, 2025

Uh oh!

dixyes commented Jun 29, 2025 •

edited

Loading

Uh oh!

Uh oh!


		declare(strict_types=1);

		require __DIR__ . '/gh18953/autoload.inc';

Fix stream double free in phar #18953

Are you sure you want to change the base?

Fix stream double free in phar #18953

Conversation

dixyes commented Jun 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dixyes commented Jun 29, 2025

Uh oh!

dixyes commented Jun 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

dixyes commented Jun 29, 2025 •

edited

Loading