Skip to content

Commit 07ddc09

Browse files
arnaud-lbarnaud-lb
committed
Prevent ASSIGN_OP / ASSIGN_DIM_OP lvalue from being released during assignment
In ASSIGN_DIM_OP, side effects of coercion may release the array or make the dimension pointer invalid (by reallocating the array). Increasing the array's refcount around the binary op is enough to prevent both issues. In ASSIGN_OP, if the variable is a reference, side effects may release it. Again, increasing the refcount prevents this.
1 parent 76e26c6 commit 07ddc09

File tree

6 files changed

+351
-56
lines changed

6 files changed

+351
-56
lines changed

‎Zend/tests/gh20319-001.phpt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-20319 001: ASSIGN_OP: Ref lvalue may be released by __toString()
3+
--ENV--
4+
LEN=10
5+
--FILE--
6+
<?php
7+
8+
$b = &$c;
9+
var_dump($b .= new class {
10+
function __toString() {
11+
unset($GLOBALS['b'], $GLOBALS['c']);
12+
return str_repeat('d', (int)getenv('LEN'));
13+
}
14+
});
15+
16+
var_dump(isset($b));
17+
18+
$b = &$c;
19+
$b .= new class {
20+
function __toString() {
21+
unset($GLOBALS['b'], $GLOBALS['c']);
22+
return str_repeat('d', (int)getenv('LEN'));
23+
}
24+
};
25+
26+
var_dump(isset($b));
27+
28+
?>
29+
--EXPECT--
30+
string(10) "dddddddddd"
31+
bool(false)
32+
bool(false)

‎Zend/tests/gh20319-002.phpt‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
GH-20319 002: ASSIGN_OP: var-var ref lvalue may be released by __toString()
3+
--ENV--
4+
LEN=10
5+
--FILE--
6+
<?php
7+
8+
$b = &$c;
9+
$v = 'b';
10+
var_dump($$v .= new class {
11+
function __toString() {
12+
unset($GLOBALS['b'], $GLOBALS['c']);
13+
return str_repeat('d', (int)getenv('LEN'));
14+
}
15+
});
16+
17+
var_dump(isset($b));
18+
19+
$b = &$c;
20+
$$v .= new class {
21+
function __toString() {
22+
unset($GLOBALS['b'], $GLOBALS['c']);
23+
return str_repeat('d', (int)getenv('LEN'));
24+
}
25+
};
26+
27+
var_dump(isset($b));
28+
29+
?>
30+
--EXPECT--
31+
string(10) "dddddddddd"
32+
bool(false)
33+
bool(false)

‎Zend/tests/gh20319-003.phpt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
GH-20319 003: ASSIGN_DIM_OP: Array lvalue may be released by __toString()
3+
--ENV--
4+
LEN=10
5+
--FILE--
6+
<?php
7+
8+
$b = ['test'];
9+
10+
var_dump($b[0] .= new class {
11+
function __toString() {
12+
unset($GLOBALS['b']);
13+
return str_repeat('d', (int)getenv('LEN'));
14+
}
15+
});
16+
17+
var_dump(isset($b));
18+
19+
unset($b);
20+
21+
$b = ['test'];
22+
23+
$b[0] .= new class {
24+
function __toString() {
25+
unset($GLOBALS['b']);
26+
return str_repeat('d', (int)getenv('LEN'));
27+
}
28+
};
29+
30+
var_dump(isset($b));
31+
32+
?>
33+
--EXPECT--
34+
string(14) "testdddddddddd"
35+
bool(false)
36+
bool(false)

‎Zend/tests/gh20319-004.phpt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
GH-20319 004: ASSIGN_DIM_OP: Array dim lvalue may be released by __toString()
3+
--FILE--
4+
<?php
5+
6+
$t = 'test';
7+
$b = [&$t];
8+
9+
var_dump($b[0] .= new class {
10+
function __toString() {
11+
global $b;
12+
unset($b[0]);
13+
return str_repeat('d', (int)getenv('LEN'));
14+
}
15+
});
16+
17+
var_dump($b);
18+
19+
unset($b);
20+
unset($t);
21+
22+
$t = 'test';
23+
$b = [&$t];
24+
25+
$b[0] .= new class {
26+
function __toString() {
27+
global $b;
28+
unset($b[0]);
29+
return str_repeat('d', (int)getenv('LEN'));
30+
}
31+
};
32+
33+
var_dump($b);
34+
35+
?>
36+
--EXPECT--
37+
string(4) "test"
38+
array(0) {
39+
}
40+
array(0) {
41+
}

‎Zend/zend_vm_def.h‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,9 @@ ZEND_VM_C_LABEL(assign_dim_op_new_array):
11831183

11841184
value = get_op_data_zval_ptr_r((opline+1)->op1_type, (opline+1)->op1);
11851185

1186+
/* Prevents array from being released or updated during binary_op */
1187+
GC_ADDREF(ht);
1188+
11861189
do {
11871190
if (OP2_TYPE != IS_UNUSED && UNEXPECTED(Z_ISREF_P(var_ptr))) {
11881191
zend_reference *ref = Z_REF_P(var_ptr);
@@ -1198,6 +1201,9 @@ ZEND_VM_C_LABEL(assign_dim_op_new_array):
11981201
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
11991202
ZVAL_COPY(EX_VAR(opline->result.var), var_ptr);
12001203
}
1204+
1205+
GC_DTOR(ht);
1206+
12011207
FREE_OP((opline+1)->op1_type, (opline+1)->op1.var);
12021208
} else {
12031209
if (EXPECTED(Z_ISREF_P(container))) {
@@ -1259,22 +1265,45 @@ ZEND_VM_HANDLER(26, ZEND_ASSIGN_OP, VAR|CV, CONST|TMPVAR|CV, OP)
12591265
value = GET_OP2_ZVAL_PTR(BP_VAR_R);
12601266
var_ptr = GET_OP1_ZVAL_PTR_PTR(BP_VAR_RW);
12611267

1262-
do {
1263-
if (UNEXPECTED(Z_TYPE_P(var_ptr) == IS_REFERENCE)) {
1264-
zend_reference *ref = Z_REF_P(var_ptr);
1265-
var_ptr = Z_REFVAL_P(var_ptr);
1266-
if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(ref))) {
1267-
zend_binary_assign_op_typed_ref(ref, value OPLINE_CC EXECUTE_DATA_CC);
1268-
break;
1268+
if (UNEXPECTED(Z_TYPE_P(var_ptr) == IS_REFERENCE)) {
1269+
ZEND_VM_C_LABEL(assign_op_ref):
1270+
zend_reference *ref = Z_REF_P(var_ptr);
1271+
GC_ADDREF(ref);
1272+
1273+
var_ptr = Z_REFVAL_P(var_ptr);
1274+
if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(ref))) {
1275+
zend_binary_assign_op_typed_ref(ref, value OPLINE_CC EXECUTE_DATA_CC);
1276+
} else {
1277+
zend_binary_op(var_ptr, var_ptr, value OPLINE_CC);
1278+
}
1279+
1280+
if (UNEXPECTED(GC_DELREF(ref) == 0)) {
1281+
if (RETURN_VALUE_USED(opline)) {
1282+
ZVAL_COPY_VALUE(EX_VAR(opline->result.var), var_ptr);
1283+
var_ptr = EX_VAR(opline->result.var);
1284+
} else {
1285+
zval_ptr_dtor(var_ptr);
12691286
}
1287+
efree_size(ref, sizeof(zend_reference));
1288+
ZEND_VM_C_GOTO(assign_op_end);
1289+
}
1290+
} else {
1291+
if (OP1_TYPE == IS_VAR) {
1292+
ZEND_ASSERT(Z_TYPE_P(EX_VAR(opline->op1.num)) == IS_INDIRECT);
1293+
/* op1 is a var-var, var_ptr is a symbol table slot which may be
1294+
* invalidated by the binary operation. Turn it into a ref so we
1295+
* control the lifetime of the zval slot. */
1296+
ZVAL_MAKE_REF(var_ptr);
1297+
ZEND_VM_C_GOTO(assign_op_ref);
12701298
}
12711299
zend_binary_op(var_ptr, var_ptr, value OPLINE_CC);
1272-
} while (0);
1300+
}
12731301

12741302
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
12751303
ZVAL_COPY(EX_VAR(opline->result.var), var_ptr);
12761304
}
12771305

1306+
ZEND_VM_C_LABEL(assign_op_end):
12781307
FREE_OP2();
12791308
FREE_OP1();
12801309
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();

0 commit comments

Comments
 (0)