SplHeap/SplPriorityQueue serialization exposes INDIRECTs

[chongwick]

Description

The following code:

<?php
$v_15488 = new SplMaxHeap();
$v_15506 = 42;
$v_15490 = $v_15488->insert($v_15506,);
$v_15497 = new SplPriorityQueue();
class CustomHeap extends SplMaxHeap {
    public $flags = 'user_property';
    public $heap_elements = 'user_property';
}
$v_15505 = new CustomHeap();$v_15507 = $v_15505->insert($v_15497,);
$v_15492 = $v_15488->insert($v_15507,);
$v_15494 = $v_15488->insert($v_15494,);
$v_15495 = $v_15488->__serialize();
$v_3982 = 'f1 called\n';
$v_15496 = var_dump($v_3982,);
$v_15508 = $v_15505->__serialize();
$v_15500 = $v_15497->insert($v_15500,$v_15508,);

Resulted in this output:

Warning: Undefined variable $v_15494 in /home/w023dtc/treebugs/abcc4f872221c3e9bd59.php.er on line 12
string(11) "f1 called\n"

Warning: Undefined variable $v_15500 in /home/w023dtc/treebugs/abcc4f872221c3e9bd59.php.er on line 17
=================================================================
==2473717==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c00001ef91 at pc 0x000006443017 bp 0x7ffe907a2b50 sp 0x7ffe907a2b48
READ of size 1 at 0x60c00001ef91 thread T0
    #0 0x6443016 in gc_mark_grey /home/w023dtc/nightly_php/php-src/Zend/zend_gc.c:1173:8
    #1 0x642d59a in gc_mark_roots /home/w023dtc/nightly_php/php-src/Zend/zend_gc.c:1269:5
    #2 0x64260f6 in zend_gc_collect_cycles /home/w023dtc/nightly_php/php-src/Zend/zend_gc.c:2007:3
    #3 0x5bee307 in zend_shutdown_executor_values /home/w023dtc/nightly_php/php-src/Zend/zend_execute_API.c:429:4
    #4 0x5bf009e in shutdown_executor /home/w023dtc/nightly_php/php-src/Zend/zend_execute_API.c:457:2
    #5 0x69ec75b in zend_deactivate /home/w023dtc/nightly_php/php-src/Zend/zend.c:1351:2
    #6 0x51c4835 in php_request_shutdown /home/w023dtc/nightly_php/php-src/main/main.c:2020:2
    #7 0x6a19561 in do_cli /home/w023dtc/nightly_php/php-src/sapi/cli/php_cli.c:1158:3
    #8 0x6a0e53f in main /home/w023dtc/nightly_php/php-src/sapi/cli/php_cli.c:1362:18
    #9 0x14a3745d0d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #10 0x14a3745d0e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #11 0x607b04 in _start (/home/w023dtc/nightly_php/php-src/sapi/cli/php+0x607b04)

0x60c00001ef91 is located 81 bytes inside of 120-byte region [0x60c00001ef40,0x60c00001efb8)
freed by thread T0 here:
    #0 0x682762 in free (/home/w023dtc/nightly_php/php-src/sapi/cli/php+0x682762)
    #1 0x5841023 in __zend_free /home/w023dtc/nightly_php/php-src/Zend/zend_alloc.c:3571:2
    #2 0x584c0db in _efree /home/w023dtc/nightly_php/php-src/Zend/zend_alloc.c:2790:3
    #3 0x688dc3d in zend_objects_store_del /home/w023dtc/nightly_php/php-src/Zend/zend_objects_API.c:200:3
    #4 0x69a3937 in rc_dtor_func /home/w023dtc/nightly_php/php-src/Zend/zend_variables.c:57:2
    #5 0x69a3bbe in i_zval_ptr_dtor /home/w023dtc/nightly_php/php-src/Zend/zend_variables.h:45:4
    #6 0x69a3974 in zval_ptr_dtor /home/w023dtc/nightly_php/php-src/Zend/zend_variables.c:84:2
    #7 0x64c3da1 in _zend_hash_del_el_ex /home/w023dtc/nightly_php/php-src/Zend/zend_hash.c:1493:3
    #8 0x64c151d in _zend_hash_del_el /home/w023dtc/nightly_php/php-src/Zend/zend_hash.c:1520:2
    #9 0x64dae64 in zend_hash_reverse_apply /home/w023dtc/nightly_php/php-src/Zend/zend_hash.c:2236:5
    #10 0x5be20ac in shutdown_destructors /home/w023dtc/nightly_php/php-src/Zend/zend_execute_API.c:262:4
    #11 0x69ebedb in zend_call_destructors /home/w023dtc/nightly_php/php-src/Zend/zend.c:1336:3
    #12 0x51c29d3 in php_request_shutdown /home/w023dtc/nightly_php/php-src/main/main.c:1980:3
    #13 0x6a19561 in do_cli /home/w023dtc/nightly_php/php-src/sapi/cli/php_cli.c:1158:3
    #14 0x6a0e53f in main /home/w023dtc/nightly_php/php-src/sapi/cli/php_cli.c:1362:18
    #15 0x14a3745d0d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

previously allocated by thread T0 here:
    #0 0x6829cd in malloc (/home/w023dtc/nightly_php/php-src/sapi/cli/php+0x6829cd)
    #1 0x584d393 in __zend_malloc /home/w023dtc/nightly_php/php-src/Zend/zend_alloc.c:3543:14
    #2 0x584baf9 in _emalloc /home/w023dtc/nightly_php/php-src/Zend/zend_alloc.c:2780:10
    #3 0x3fa8ae2 in zend_object_alloc /home/w023dtc/nightly_php/php-src/Zend/zend_objects_API.h:94:14
    #4 0x3fa73e2 in spl_heap_object_new_ex /home/w023dtc/nightly_php/php-src/ext/spl/spl_heap.c:418:11
    #5 0x3fa28aa in spl_heap_object_new /home/w023dtc/nightly_php/php-src/ext/spl/spl_heap.c:478:9
    #6 0x58964bc in _object_and_properties_init /home/w023dtc/nightly_php/php-src/Zend/zend_API.c:1856:3
    #7 0x5896830 in object_init_ex /home/w023dtc/nightly_php/php-src/Zend/zend_API.c:1870:9
    #8 0x5f67681 in ZEND_NEW_SPEC_CONST_UNUSED_HANDLER /home/w023dtc/nightly_php/php-src/Zend/zend_vm_execute.h:11265:6
    #9 0x5c7732b in execute_ex /home/w023dtc/nightly_php/php-src/Zend/zend_vm_execute.h:115754:12
    #10 0x5c798bc in zend_execute /home/w023dtc/nightly_php/php-src/Zend/zend_vm_execute.h:121466:2
    #11 0x69ff249 in zend_execute_script /home/w023dtc/nightly_php/php-src/Zend/zend.c:1977:3
    #12 0x51d758a in php_execute_script_ex /home/w023dtc/nightly_php/php-src/main/main.c:2640:13
    #13 0x51d86c8 in php_execute_script /home/w023dtc/nightly_php/php-src/main/main.c:2680:9
    #14 0x6a1415a in do_cli /home/w023dtc/nightly_php/php-src/sapi/cli/php_cli.c:951:5
    #15 0x6a0e53f in main /home/w023dtc/nightly_php/php-src/sapi/cli/php_cli.c:1362:18
    #16 0x14a3745d0d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16

SUMMARY: AddressSanitizer: heap-use-after-free /home/w023dtc/nightly_php/php-src/Zend/zend_gc.c:1173:8 in gc_mark_grey
Shadow bytes around the buggy address:
  0x0c187fffbda0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c187fffbdb0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c187fffbdc0: fd fd fd fd fd fd fd fd fa fa fa fa fa fa fa fa
  0x0c187fffbdd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c187fffbde0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x0c187fffbdf0: fd fd[fd]fd fd fd fd fa fa fa fa fa fa fa fa fa
  0x0c187fffbe00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fffbe10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fffbe20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fffbe30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c187fffbe40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Shadow gap:              cc
==2473717==ABORTING

USE_ZEND_ALLOC=0 php script.php

PHP Version

nightly

Operating System

ubuntu 22.04

[nielsdos]

It's related due to the serialization. added in 8.5(-dev). Even the following already produces a wrong result:

class CustomHeap extends SplMaxHeap {
    public $field = 0;
}
$heap = new CustomHeap();
$data = $heap->__serialize();
var_dump($data);

[nielsdos]

So the above issue happens because it exposes INDIRECTs to userland which should never happen.
This is likely wrong in Randomizer too.

The following patch would fix the __serialize call:

diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c
index 19e86b0d80b..cb127cf8c77 100644
--- a/ext/spl/spl_heap.c
+++ b/ext/spl/spl_heap.c
@@ -1185,8 +1185,7 @@ PHP_METHOD(SplPriorityQueue, __serialize)
 
 	array_init(return_value);
 
-	ZVAL_ARR(&props, zend_std_get_properties(&intern->std));
-	Z_TRY_ADDREF(props);
+	ZVAL_ARR(&props, zend_array_dup(zend_std_get_properties(&intern->std)));
 	zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &props);
 
 	spl_heap_serialize_internal_state(&state, intern, true);
@@ -1257,8 +1256,7 @@ PHP_METHOD(SplHeap, __serialize)
 
 	array_init(return_value);
 
-	ZVAL_ARR(&props, zend_std_get_properties(&intern->std));
-	Z_TRY_ADDREF(props);
+	ZVAL_ARR(&props, zend_array_dup(zend_std_get_properties(&intern->std)));
 	zend_hash_next_index_insert(Z_ARRVAL_P(return_value), &props);
 
 	spl_heap_serialize_internal_state(&state, intern, false);

And it turns out that's actually the root cause.