Fix GH-10168: heap-buffer-overflow at zval_undefined_cv

Zend/tests/gh10168_1.phpt

@@ -0,0 +1,32 @@
+--TEST--
+GH-10168 (heap-buffer-overflow at zval_undefined_cv): array variation
+--FILE--
+<?php
+
+class Test
+{
+    static $instances;
+    public function __construct() {
+	    (self::$instances[NULL] = $this) > 0;
+	    var_dump(self::$instances);
+    }
+
+    function __destruct() {
+        unset(self::$instances[NULL]);
+    }
+}
+new Test();
+new Test();
+
+?>
+--EXPECTF--
+Notice: Object of class Test could not be converted to int in %s on line %d
+array(1) {
+  [""]=>
+  object(Test)#1 (0) {
+  }
+}
+
+Notice: Object of class Test could not be converted to int in %s on line %d
+array(0) {
+}

Zend/tests/gh10168_2.phpt

@@ -0,0 +1,31 @@
+--TEST--
+GH-10168 (heap-buffer-overflow at zval_undefined_cv): assign global variation
+--FILE--
+<?php
+
+$a = null;
+
+class Test
+{
+    public function __construct() {
+	    ($GLOBALS['a'] = $this) > 0;
+	    var_dump($GLOBALS['a']);
+    }
+
+    function __destruct() {
+        unset($GLOBALS['a']);
+    }
+}
+new Test();
+new Test();
+
+?>
+--EXPECTF--
+Notice: Object of class Test could not be converted to int in %s on line %d
+object(Test)#1 (0) {
+}
+
+Notice: Object of class Test could not be converted to int in %s on line %d
+
+Warning: Undefined global variable $a in %s on line %d
+NULL

Zend/zend_execute.h

@@ -137,14 +137,31 @@ static zend_always_inline void zend_copy_to_variable(zval *variable_ptr, zval *v
 	}
 }
 
-static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict)
+/* Handles the garbage when using zend_assign_to_variable_delay_garbage_handling */
+static zend_always_inline void zend_handle_garbage_from_variable_assignment(zend_refcounted *garbage)
+{
+	if (!garbage)
+		return;
+	if (GC_DELREF(garbage) == 0) {
+		rc_dtor_func(garbage);
+	} else { /* we need to split */
+		/* optimized version of GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) */
+		if (UNEXPECTED(GC_MAY_LEAK(garbage))) {
+			gc_possible_root(garbage);
+		}
+	}
+}
+
+/* This is the same as zend_assign_to_variable() but without the garbage handling, this is left for the caller to do using zend_handle_garbage_from_variable_assignment().
+ * The reason one might want to delay garbage handling is because the garbage handling might destroy the newly returned value while
+ * that value was supposed to be used in a result. Using zend_assign_to_variable() instead in that case would result in a potential use-after-free. */
+static zend_always_inline zval* zend_assign_to_variable_delay_garbage_handling(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict, zend_refcounted **garbage)
 {
 	do {
 		if (UNEXPECTED(Z_REFCOUNTED_P(variable_ptr))) {
-			zend_refcounted *garbage;
-
 			if (Z_ISREF_P(variable_ptr)) {
 				if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(variable_ptr)))) {
+					*garbage = NULL;
 					return zend_assign_to_typed_ref(variable_ptr, value, value_type, strict);
 				}
 
@@ -153,24 +170,25 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval
 					break;
 				}
 			}
-			garbage = Z_COUNTED_P(variable_ptr);
+			*garbage = Z_COUNTED_P(variable_ptr);
 			zend_copy_to_variable(variable_ptr, value, value_type);
-			if (GC_DELREF(garbage) == 0) {
-				rc_dtor_func(garbage);
-			} else { /* we need to split */
-				/* optimized version of GC_ZVAL_CHECK_POSSIBLE_ROOT(variable_ptr) */
-				if (UNEXPECTED(GC_MAY_LEAK(garbage))) {
-					gc_possible_root(garbage);
-				}
-			}
 			return variable_ptr;
 		}
 	} while (0);
 
+	*garbage = NULL;
 	zend_copy_to_variable(variable_ptr, value, value_type);
 	return variable_ptr;
 }
 
+static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict)
+{
+	zend_refcounted *garbage;
+	variable_ptr = zend_assign_to_variable_delay_garbage_handling(variable_ptr, value, value_type, strict, &garbage);
+	zend_handle_garbage_from_variable_assignment(garbage);
+	return variable_ptr;
+}
+
 ZEND_API zend_result ZEND_FASTCALL zval_update_constant(zval *pp);
 ZEND_API zend_result ZEND_FASTCALL zval_update_constant_ex(zval *pp, zend_class_entry *scope);
 

Zend/zend_vm_def.h

@@ -2577,6 +2577,9 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
 					Z_ADDREF_P(value);
 				}
 			}
+			if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
+				ZVAL_COPY(EX_VAR(opline->result.var), value);
+			}
 		} else {
 			dim = GET_OP2_ZVAL_PTR_UNDEF(BP_VAR_R);
 			if (OP2_TYPE == IS_CONST) {
@@ -2588,10 +2591,12 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
 				ZEND_VM_C_GOTO(assign_dim_error);
 			}
 			value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);
-			value = zend_assign_to_variable(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
-		}
-		if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
-			ZVAL_COPY(EX_VAR(opline->result.var), value);
+			zend_refcounted *garbage;
+			value = zend_assign_to_variable_delay_garbage_handling(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);
+			if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
+				ZVAL_COPY(EX_VAR(opline->result.var), value);
+			}
+			zend_handle_garbage_from_variable_assignment(garbage);
 		}
 	} else {
 		if (EXPECTED(Z_ISREF_P(object_ptr))) {
@@ -2685,12 +2690,15 @@ ZEND_VM_HANDLER(22, ZEND_ASSIGN, VAR|CV, CONST|TMP|VAR|CV, SPEC(RETVAL))
 	value = GET_OP2_ZVAL_PTR(BP_VAR_R);
 	variable_ptr = GET_OP1_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
 
-	value = zend_assign_to_variable(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
+	zend_refcounted *garbage;
+	value = zend_assign_to_variable_delay_garbage_handling(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES(), &garbage);
 	if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
 		ZVAL_COPY(EX_VAR(opline->result.var), value);
 	}
+	zend_handle_garbage_from_variable_assignment(garbage);
+
 	FREE_OP1_VAR_PTR();
-	/* zend_assign_to_variable() always takes care of op2, never free it! */
+	/* zend_assign_to_variable_*() always takes care of op2, never free it! */
 
 	ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
 }