Skip to content

Commit 8d2df86

Browse files
realFlowControlFlorian Engelhardt
realFlowControl
and
Florian Engelhardt
authored
Fix invalid opline in OOM handlers within ZEND_FUNC_GET_ARGS and ZEND_BIND_STATIC (#12768)
* fix segfault in `ZEND_BIND_STATIC` In case a `ZEND_BIND_STATIC` is being executed, while the current chunk is full, the `zend_array_dup()` call will trigger a OOM in ZendMM which will crash, as the opline might be a dangling pointer. * add missing test * `assert()`ing seems easier than trying to make the compiler to not optimize * moved from function call to INI setting, so we can use this in other places as well * make `assert()` work no NDEBUG builds * document magic number * fix segfault in `ZEND_FUNC_GET_ARGS` In case a `ZEND_FUNC_GET_ARGS` is being executed, while the current chunk is full, the `zend_new_array()` call will trigger a OOM in ZendMM which will crash, as the opline might be a dangling pointer. --------- Co-authored-by: Florian Engelhardt <[email protected]>
1 parent 87107f8 commit 8d2df86

File tree

6 files changed

+156
-2
lines changed

6 files changed

+156
-2
lines changed

‎Zend/zend_vm_def.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8788,6 +8788,8 @@ ZEND_VM_HANDLER(183, ZEND_BIND_STATIC, CV, UNUSED, REF)
87888788

87898789
variable_ptr = GET_OP1_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
87908790

8791+
SAVE_OPLINE();
8792+
87918793
ht = ZEND_MAP_PTR_GET(EX(func)->op_array.static_variables_ptr);
87928794
if (!ht) {
87938795
ht = zend_array_dup(EX(func)->op_array.static_variables);
@@ -8797,7 +8799,6 @@ ZEND_VM_HANDLER(183, ZEND_BIND_STATIC, CV, UNUSED, REF)
87978799

87988800
value = (zval*)((char*)ht->arData + (opline->extended_value & ~(ZEND_BIND_REF|ZEND_BIND_IMPLICIT|ZEND_BIND_EXPLICIT)));
87998801

8800-
SAVE_OPLINE();
88018802
if (opline->extended_value & ZEND_BIND_REF) {
88028803
if (Z_TYPE_P(value) == IS_CONSTANT_AST) {
88038804
if (UNEXPECTED(zval_update_constant_ex(value, EX(func)->op_array.scope) != SUCCESS)) {
@@ -9250,6 +9251,7 @@ ZEND_VM_HANDLER(172, ZEND_FUNC_GET_ARGS, UNUSED|CONST, UNUSED)
92509251
}
92519252

92529253
if (result_size) {
9254+
SAVE_OPLINE();
92539255
uint32_t first_extra_arg = EX(func)->op_array.num_args;
92549256

92559257
ht = zend_new_array(result_size);

‎Zend/zend_vm_execute.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10700,6 +10700,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FUNC_GET_ARGS_SPEC_CONST_UNUSE
1070010700
}
1070110701

1070210702
if (result_size) {
10703+
SAVE_OPLINE();
1070310704
uint32_t first_extra_arg = EX(func)->op_array.num_args;
1070410705

1070510706
ht = zend_new_array(result_size);
@@ -36064,6 +36065,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_FUNC_GET_ARGS_SPEC_UNUSED_UNUS
3606436065
}
3606536066

3606636067
if (result_size) {
36068+
SAVE_OPLINE();
3606736069
uint32_t first_extra_arg = EX(func)->op_array.num_args;
3606836070

3606936071
ht = zend_new_array(result_size);
@@ -48471,6 +48473,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_BIND_STATIC_SPEC_CV_UNUSED_HAN
4847148473

4847248474
variable_ptr = EX_VAR(opline->op1.var);
4847348475

48476+
SAVE_OPLINE();
48477+
4847448478
ht = ZEND_MAP_PTR_GET(EX(func)->op_array.static_variables_ptr);
4847548479
if (!ht) {
4847648480
ht = zend_array_dup(EX(func)->op_array.static_variables);
@@ -48480,7 +48484,6 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_BIND_STATIC_SPEC_CV_UNUSED_HAN
4848048484

4848148485
value = (zval*)((char*)ht->arData + (opline->extended_value & ~(ZEND_BIND_REF|ZEND_BIND_IMPLICIT|ZEND_BIND_EXPLICIT)));
4848248486

48483-
SAVE_OPLINE();
4848448487
if (opline->extended_value & ZEND_BIND_REF) {
4848548488
if (Z_TYPE_P(value) == IS_CONSTANT_AST) {
4848648489
if (UNEXPECTED(zval_update_constant_ex(value, EX(func)->op_array.scope) != SUCCESS)) {

‎ext/zend_test/php_test.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
5252
HashTable global_weakmap;
5353
int replace_zend_execute_ex;
5454
int register_passes;
55+
int observe_opline_in_zendmm;
56+
zend_mm_heap* zend_orig_heap;
57+
zend_mm_heap* zend_test_heap;
5558
zend_test_fiber *active_fiber;
5659
ZEND_END_MODULE_GLOBALS(zend_test)
5760

‎ext/zend_test/test.c

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,15 @@
3030
#include "zend_interfaces.h"
3131
#include "zend_weakrefs.h"
3232
#include "Zend/Optimizer/zend_optimizer.h"
33+
#include "Zend/zend_alloc.h"
3334
#include "test_arginfo.h"
3435

36+
// `php.h` sets `NDEBUG` when not `PHP_DEBUG` which will make `assert()` from
37+
// assert.h a no-op. In order to have `assert()` working on NDEBUG builds, we
38+
// undefine `NDEBUG` and re-include assert.h
39+
#undef NDEBUG
40+
#include "assert.h"
41+
3542
#if defined(HAVE_LIBXML) && !defined(PHP_WIN32)
3643
# include <libxml/globals.h>
3744
# include <libxml/parser.h>
@@ -364,6 +371,68 @@ static ZEND_FUNCTION(zend_test_crash)
364371
php_printf("%s", invalid);
365372
}
366373

374+
static bool has_opline(zend_execute_data *execute_data)
375+
{
376+
return execute_data
377+
&& execute_data->func
378+
&& ZEND_USER_CODE(execute_data->func->type)
379+
&& execute_data->opline
380+
;
381+
}
382+
383+
void * zend_test_custom_malloc(size_t len)
384+
{
385+
if (has_opline(EG(current_execute_data))) {
386+
assert(EG(current_execute_data)->opline->lineno != (uint32_t)-1);
387+
}
388+
return _zend_mm_alloc(ZT_G(zend_orig_heap), len ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
389+
}
390+
391+
void zend_test_custom_free(void *ptr)
392+
{
393+
if (has_opline(EG(current_execute_data))) {
394+
assert(EG(current_execute_data)->opline->lineno != (uint32_t)-1);
395+
}
396+
_zend_mm_free(ZT_G(zend_orig_heap), ptr ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
397+
}
398+
399+
void * zend_test_custom_realloc(void * ptr, size_t len)
400+
{
401+
if (has_opline(EG(current_execute_data))) {
402+
assert(EG(current_execute_data)->opline->lineno != (uint32_t)-1);
403+
}
404+
return _zend_mm_realloc(ZT_G(zend_orig_heap), ptr, len ZEND_FILE_LINE_EMPTY_CC ZEND_FILE_LINE_EMPTY_CC);
405+
}
406+
407+
static PHP_INI_MH(OnUpdateZendTestObserveOplineInZendMM)
408+
{
409+
if (new_value == NULL) {
410+
return FAILURE;
411+
}
412+
413+
int int_value = zend_ini_parse_bool(new_value);
414+
415+
if (int_value == 1) {
416+
// `zend_mm_heap` is a private struct, so we have not way to find the
417+
// actual size, but 4096 bytes should be enough
418+
ZT_G(zend_test_heap) = malloc(4096);
419+
memset(ZT_G(zend_test_heap), 0, 4096);
420+
zend_mm_set_custom_handlers(
421+
ZT_G(zend_test_heap),
422+
zend_test_custom_malloc,
423+
zend_test_custom_free,
424+
zend_test_custom_realloc
425+
);
426+
ZT_G(zend_orig_heap) = zend_mm_get_heap();
427+
zend_mm_set_heap(ZT_G(zend_test_heap));
428+
} else if (ZT_G(zend_test_heap)) {
429+
free(ZT_G(zend_test_heap));
430+
ZT_G(zend_test_heap) = NULL;
431+
zend_mm_set_heap(ZT_G(zend_orig_heap));
432+
}
433+
return OnUpdateBool(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
434+
}
435+
367436
static ZEND_FUNCTION(zend_test_is_pcre_bundled)
368437
{
369438
ZEND_PARSE_PARAMETERS_NONE();
@@ -558,6 +627,7 @@ static ZEND_METHOD(ZendTestChildClassWithMethodWithParameterAttribute, override)
558627
PHP_INI_BEGIN()
559628
STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals)
560629
STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals)
630+
STD_PHP_INI_BOOLEAN("zend_test.observe_opline_in_zendmm", "0", PHP_INI_ALL, OnUpdateZendTestObserveOplineInZendMM, observe_opline_in_zendmm, zend_zend_test_globals, zend_test_globals)
561631
PHP_INI_END()
562632

563633
void (*old_zend_execute_ex)(zend_execute_data *execute_data);
@@ -700,6 +770,13 @@ PHP_RSHUTDOWN_FUNCTION(zend_test)
700770
zend_weakrefs_hash_del(&ZT_G(global_weakmap), (zend_object *)(uintptr_t)objptr);
701771
} ZEND_HASH_FOREACH_END();
702772
zend_hash_destroy(&ZT_G(global_weakmap));
773+
774+
if (ZT_G(zend_test_heap)) {
775+
free(ZT_G(zend_test_heap));
776+
ZT_G(zend_test_heap) = NULL;
777+
zend_mm_set_heap(ZT_G(zend_orig_heap));
778+
}
779+
703780
return SUCCESS;
704781
}
705782

‎ext/zend_test/tests/opline_dangling.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
possible segfault in `ZEND_BIND_STATIC`
3+
--DESCRIPTION--
4+
https://github.com/php/php-src/pull/12758
5+
--EXTENSIONS--
6+
zend_test
7+
--INI--
8+
zend_test.observe_opline_in_zendmm=1
9+
--FILE--
10+
<?php
11+
12+
function &ref() {
13+
static $a = 5;
14+
return $a;
15+
}
16+
17+
class Foo {
18+
public static int $i;
19+
public static string $s = "x";
20+
}
21+
22+
var_dump(Foo::$i = "1");
23+
var_dump(Foo::$s, Foo::$i);
24+
var_dump(ref());
25+
26+
echo 'Done.';
27+
?>
28+
--EXPECT--
29+
int(1)
30+
string(1) "x"
31+
int(1)
32+
int(5)
33+
Done.

‎ext/zend_test/tests/opline_dangling_02.phpt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
possible segfault in `ZEND_FUNC_GET_ARGS`
3+
--DESCRIPTION--
4+
--EXTENSIONS--
5+
zend_test
6+
--INI--
7+
zend_test.observe_opline_in_zendmm=1
8+
--FILE--
9+
<?php
10+
11+
function ref() {
12+
return func_get_args();
13+
}
14+
15+
class Foo {
16+
public static int $i;
17+
public static string $s = "x";
18+
}
19+
20+
var_dump(Foo::$i = "1");
21+
var_dump(Foo::$s, Foo::$i);
22+
var_dump(ref('string', 0));
23+
24+
echo 'Done.';
25+
?>
26+
--EXPECT--
27+
int(1)
28+
string(1) "x"
29+
int(1)
30+
array(2) {
31+
[0]=>
32+
string(6) "string"
33+
[1]=>
34+
int(0)
35+
}
36+
Done.

0 commit comments

Comments
 (0)