Skip to content

Commit 96bf925

Browse files
nikicnikic
committed
Fix return value of wrong fucntion by-ref assign
We should be using the result of zend_assign_to_variable() here, which will deref prior to potential freeing. Fixes oss-fuzz #29899.
1 parent bdc60fa commit 96bf925

4 files changed

Lines changed: 26 additions & 24 deletions

File tree

‎Zend/tests/assign_by_val_function_by_ref_return_value.phpt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
--TEST--
2+
Return value of assigning by-val function result by-reference
3+
--FILE--
4+
<?php
5+
$a = [&$a];
6+
var_dump($a[0] =& returnsVal());
7+
function returnsVal() {}
8+
?>
9+
--EXPECTF--
10+
Notice: Only variables should be assigned by reference in %s on line %d
11+
NULL

‎Zend/zend_execute.c‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -585,18 +585,16 @@ static zend_never_inline zval* zend_assign_to_typed_property_reference(zend_prop
585585
return prop;
586586
}
587587

588-
static zend_never_inline ZEND_COLD int zend_wrong_assign_to_variable_reference(zval *variable_ptr, zval *value_ptr OPLINE_DC EXECUTE_DATA_DC)
588+
static zend_never_inline ZEND_COLD zval *zend_wrong_assign_to_variable_reference(zval *variable_ptr, zval *value_ptr OPLINE_DC EXECUTE_DATA_DC)
589589
{
590590
zend_error(E_NOTICE, "Only variables should be assigned by reference");
591591
if (UNEXPECTED(EG(exception) != NULL)) {
592-
return 0;
592+
return &EG(uninitialized_zval);
593593
}
594594

595595
/* Use IS_TMP_VAR instead of IS_VAR to avoid ISREF check */
596596
Z_TRY_ADDREF_P(value_ptr);
597-
value_ptr = zend_assign_to_variable(variable_ptr, value_ptr, IS_TMP_VAR, EX_USES_STRICT_TYPES());
598-
599-
return 1;
597+
return zend_assign_to_variable(variable_ptr, value_ptr, IS_TMP_VAR, EX_USES_STRICT_TYPES());
600598
}
601599

602600
static void zend_format_type(zend_type type, const char **part1, const char **part2) {
@@ -2948,10 +2946,8 @@ static zend_always_inline void zend_assign_to_property_reference(zval *container
29482946
(opline->extended_value & ZEND_RETURNS_FUNCTION) &&
29492947
UNEXPECTED(!Z_ISREF_P(value_ptr))) {
29502948

2951-
if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(
2952-
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) {
2953-
variable_ptr = &EG(uninitialized_zval);
2954-
}
2949+
variable_ptr = zend_wrong_assign_to_variable_reference(
2950+
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC);
29552951
} else {
29562952
zend_property_info *prop_info = NULL;
29572953

‎Zend/zend_vm_def.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,9 +2772,8 @@ ZEND_VM_HANDLER(30, ZEND_ASSIGN_REF, VAR|CV, VAR|CV, SRC)
27722772
opline->extended_value == ZEND_RETURNS_FUNCTION &&
27732773
UNEXPECTED(!Z_ISREF_P(value_ptr))) {
27742774

2775-
if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) {
2776-
variable_ptr = &EG(uninitialized_zval);
2777-
}
2775+
variable_ptr = zend_wrong_assign_to_variable_reference(
2776+
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC);
27782777
} else {
27792778
zend_assign_to_variable_reference(variable_ptr, value_ptr);
27802779
}

‎Zend/zend_vm_execute.h‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26766,9 +26766,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_VAR_VAR_HANDLE
2676626766
opline->extended_value == ZEND_RETURNS_FUNCTION &&
2676726767
UNEXPECTED(!Z_ISREF_P(value_ptr))) {
2676826768

26769-
if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) {
26770-
variable_ptr = &EG(uninitialized_zval);
26771-
}
26769+
variable_ptr = zend_wrong_assign_to_variable_reference(
26770+
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC);
2677226771
} else {
2677326772
zend_assign_to_variable_reference(variable_ptr, value_ptr);
2677426773
}
@@ -29698,9 +29697,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_VAR_CV_HANDLER
2969829697
opline->extended_value == ZEND_RETURNS_FUNCTION &&
2969929698
UNEXPECTED(!Z_ISREF_P(value_ptr))) {
2970029699

29701-
if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) {
29702-
variable_ptr = &EG(uninitialized_zval);
29703-
}
29700+
variable_ptr = zend_wrong_assign_to_variable_reference(
29701+
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC);
2970429702
} else {
2970529703
zend_assign_to_variable_reference(variable_ptr, value_ptr);
2970629704
}
@@ -45451,9 +45449,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_CV_VAR_HANDLER
4545145449
opline->extended_value == ZEND_RETURNS_FUNCTION &&
4545245450
UNEXPECTED(!Z_ISREF_P(value_ptr))) {
4545345451

45454-
if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) {
45455-
variable_ptr = &EG(uninitialized_zval);
45456-
}
45452+
variable_ptr = zend_wrong_assign_to_variable_reference(
45453+
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC);
4545745454
} else {
4545845455
zend_assign_to_variable_reference(variable_ptr, value_ptr);
4545945456
}
@@ -49507,9 +49504,8 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_ASSIGN_REF_SPEC_CV_CV_HANDLER(
4950749504
opline->extended_value == ZEND_RETURNS_FUNCTION &&
4950849505
UNEXPECTED(!Z_ISREF_P(value_ptr))) {
4950949506

49510-
if (UNEXPECTED(!zend_wrong_assign_to_variable_reference(variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC))) {
49511-
variable_ptr = &EG(uninitialized_zval);
49512-
}
49507+
variable_ptr = zend_wrong_assign_to_variable_reference(
49508+
variable_ptr, value_ptr OPLINE_CC EXECUTE_DATA_CC);
4951349509
} else {
4951449510
zend_assign_to_variable_reference(variable_ptr, value_ptr);
4951549511
}

0 commit comments

Comments
 (0)