Skip to content

Commit 5f2757d

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 336fbf0 commit 5f2757d

File tree

7 files changed

+677
-104
lines changed

7 files changed

+677
-104
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/tests/gh20319-005.phpt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
GH-20319 005: ASSIGN_OP: var-var slot may be invalidated by __toString()
3+
--ENV--
4+
LEN=10
5+
--FILE--
6+
<?php
7+
8+
$v = 'b';
9+
10+
$$v = 'test';
11+
var_dump($$v .= new class {
12+
function __toString() {
13+
for ($i = 0; $i < 128; $i++) {
14+
$GLOBALS['_' . $i] = 0;
15+
}
16+
return str_repeat('a', (int)getenv('LEN'));
17+
}
18+
});
19+
20+
$$v = 'test';
21+
$$v .= new class {
22+
function __toString() {
23+
for ($i = 128; $i < 512; $i++) {
24+
$GLOBALS['_' . $i] = 0;
25+
}
26+
return str_repeat('a', (int)getenv('LEN'));
27+
}
28+
};
29+
30+
var_dump($$v);
31+
32+
?>
33+
--EXPECT--
34+
string(14) "testaaaaaaaaaa"
35+
string(14) "testaaaaaaaaaa"

‎Zend/zend_vm_def.h‎

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,6 +1188,9 @@ ZEND_VM_C_LABEL(assign_dim_op_new_array):
11881188

11891189
value = get_op_data_zval_ptr_r((opline+1)->op1_type, (opline+1)->op1);
11901190

1191+
/* Prevents array from being released or updated during binary_op */
1192+
GC_ADDREF(ht);
1193+
11911194
do {
11921195
if (OP2_TYPE != IS_UNUSED && UNEXPECTED(Z_ISREF_P(var_ptr))) {
11931196
zend_reference *ref = Z_REF_P(var_ptr);
@@ -1203,6 +1206,9 @@ ZEND_VM_C_LABEL(assign_dim_op_new_array):
12031206
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
12041207
ZVAL_COPY(EX_VAR(opline->result.var), var_ptr);
12051208
}
1209+
1210+
GC_DTOR(ht);
1211+
12061212
FREE_OP((opline+1)->op1_type, (opline+1)->op1.var);
12071213
} else {
12081214
if (EXPECTED(Z_ISREF_P(container))) {
@@ -1264,22 +1270,44 @@ ZEND_VM_HANDLER(26, ZEND_ASSIGN_OP, VAR|CV, CONST|TMPVAR|CV, OP)
12641270
value = GET_OP2_ZVAL_PTR(BP_VAR_R);
12651271
var_ptr = GET_OP1_ZVAL_PTR_PTR(BP_VAR_RW);
12661272

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

12791306
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
12801307
ZVAL_COPY(EX_VAR(opline->result.var), var_ptr);
12811308
}
12821309

1310+
ZEND_VM_C_LABEL(assign_op_end):
12831311
FREE_OP2();
12841312
FREE_OP1();
12851313
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();

0 commit comments

Comments
 (0)