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

ndossche · iluuu1994 · commit 71ddede5655f · 2023-02-08T01:06:50.000+01:00
The problem is that we're using the variable_ptr in the opcode handler *after* it has already been destroyed. The solution is to create a specialised version of zend_assign_to_variable which takes in two destination zval pointers. Closes phpGH-10524
diff --git a/NEWS b/NEWS
@@ -10,6 +10,8 @@ PHP                                                                        NEWS
     Generator emits an unavoidable fatal error or crashes). (Arnaud)
   . Fixed bug GH-10437 (Segfault/assertion when using fibers in shutdown
     function after bailout). (trowski)
+  . Fixed bug GH-10168: use-after-free when utilizing assigned object freed
+    during assignment. (nielsdos)
 
 - Date:
   . Fix GH-10447 ('p' format specifier does not yield 'Z' for 00:00). (Derick)
diff --git a/Zend/tests/gh10168_1.phpt b/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) {
+}
diff --git a/Zend/tests/gh10168_2.phpt b/Zend/tests/gh10168_2.phpt
@@ -0,0 +1,32 @@
+--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;
+		// Destructor called after comparison, so a will be NULL
+		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
diff --git a/Zend/tests/gh10168_3.phpt b/Zend/tests/gh10168_3.phpt
@@ -0,0 +1,29 @@
+--TEST--
+GH-10168 (heap-buffer-overflow at zval_undefined_cv): assign typed prop
+--FILE--
+<?php
+class Test
+{
+	static ?Test $a = null;
+
+	public function __construct() {
+		if (self::$a === null) {
+			var_dump(self::$a = &$this);
+		} else {
+			var_dump(self::$a = $this);
+		}
+	}
+
+	function __destruct() {
+		self::$a = null;
+	}
+}
+new Test();
+new Test();
+
+?>
+--EXPECTF--
+object(Test)#1 (0) {
+}
+object(Test)#2 (0) {
+}
diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -3562,7 +3562,7 @@ static zend_always_inline void i_zval_ptr_dtor_noref(zval *zval_ptr) {
 	}
 }
 
-ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict)
+ZEND_API zval* zend_assign_to_typed_ref_and_result(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict, zval *result_variable_ptr)
 {
 	bool ret;
 	zval value;
@@ -3582,6 +3582,9 @@ ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, ze
 	} else {
 		zval_ptr_dtor_nogc(&value);
 	}
+	if (result_variable_ptr) {
+		ZVAL_COPY(result_variable_ptr, variable_ptr);
+	}
 	if (value_type & (IS_VAR|IS_TMP_VAR)) {
 		if (UNEXPECTED(ref)) {
 			if (UNEXPECTED(GC_DELREF(ref) == 0)) {
@@ -3595,6 +3598,11 @@ ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, ze
 	return variable_ptr;
 }
 
+ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict)
+{
+	return zend_assign_to_typed_ref_and_result(variable_ptr, orig_value, value_type, strict, NULL);
+}
+
 ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info *prop_info, zval *orig_val, bool strict) {
 	zval *val = orig_val;
 	if (Z_ISREF_P(val) && ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(val))) {
diff --git a/Zend/zend_execute.h b/Zend/zend_execute.h
@@ -108,6 +108,7 @@ ZEND_API bool zend_verify_internal_return_type(zend_function *zf, zval *ret);
 ZEND_API void ZEND_FASTCALL zend_ref_add_type_source(zend_property_info_source_list *source_list, zend_property_info *prop);
 ZEND_API void ZEND_FASTCALL zend_ref_del_type_source(zend_property_info_source_list *source_list, zend_property_info *prop);
 
+ZEND_API zval* zend_assign_to_typed_ref_and_result(zval *variable_ptr, zval *orig_value, zend_uchar value_type, bool strict, zval *result_variable_ptr);
 ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict);
 
 static zend_always_inline void zend_copy_to_variable(zval *variable_ptr, zval *value, zend_uchar value_type)
@@ -137,12 +138,22 @@ static zend_always_inline void zend_copy_to_variable(zval *variable_ptr, zval *v
 	}
 }
 
+static zend_always_inline void zend_handle_garbage_from_variable_assignment(zend_refcounted *garbage)
+{
+	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);
+		}
+	}
+}
+
 static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict)
 {
 	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)))) {
 					return zend_assign_to_typed_ref(variable_ptr, value, value_type, strict);
@@ -153,21 +164,42 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval
 					break;
 				}
 			}
-			garbage = Z_COUNTED_P(variable_ptr);
+			zend_refcounted *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);
+			zend_handle_garbage_from_variable_assignment(garbage);
+			return variable_ptr;
+		}
+	} while (0);
+
+	zend_copy_to_variable(variable_ptr, value, value_type);
+	return variable_ptr;
+}
+
+static zend_always_inline zval* zend_assign_to_two_variables(zval *result_variable_ptr, zval *variable_ptr, zval *value, zend_uchar value_type, bool strict)
+{
+	do {
+		if (UNEXPECTED(Z_REFCOUNTED_P(variable_ptr))) {
+			if (Z_ISREF_P(variable_ptr)) {
+				if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(variable_ptr)))) {
+					variable_ptr = zend_assign_to_typed_ref_and_result(variable_ptr, value, value_type, strict, result_variable_ptr);
+					return variable_ptr;
+				}
+
+				variable_ptr = Z_REFVAL_P(variable_ptr);
+				if (EXPECTED(!Z_REFCOUNTED_P(variable_ptr))) {
+					break;
 				}
 			}
+			zend_refcounted *garbage = Z_COUNTED_P(variable_ptr);
+			zend_copy_to_variable(variable_ptr, value, value_type);
+			ZVAL_COPY(result_variable_ptr, variable_ptr);
+			zend_handle_garbage_from_variable_assignment(garbage);
 			return variable_ptr;
 		}
 	} while (0);
 
 	zend_copy_to_variable(variable_ptr, value, value_type);
+	ZVAL_COPY(result_variable_ptr, variable_ptr);
 	return variable_ptr;
 }
 
diff --git a/Zend/zend_vm_def.h b/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,11 @@ 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);
+			if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
+				zend_assign_to_two_variables(EX_VAR(opline->result.var), variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
+			} else {
+				zend_assign_to_variable(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
+			}
 		}
 	} else {
 		if (EXPECTED(Z_ISREF_P(object_ptr))) {
@@ -2685,12 +2689,14 @@ 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());
 	if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
-		ZVAL_COPY(EX_VAR(opline->result.var), value);
+		zend_assign_to_two_variables(EX_VAR(opline->result.var), variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
+	} else {
+		zend_assign_to_variable(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
 	}
+
 	FREE_OP1_VAR_PTR();
-	/* zend_assign_to_variable() always takes care of op2, never free it! */
+	/* zend_assign_to_(two_)variable(s)() always takes care of op2, never free it! */
 
 	ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
 }
diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h

Original file line number	Diff line number	Diff line change
`@@ -3562,7 +3562,7 @@ static zend_always_inline void i_zval_ptr_dtor_noref(zval *zval_ptr) {`
`3562`	`3562`	`}`
`3563`	`3563`	`}`
`3564`	`3564`
`3565`		`-ZEND_API zval* zend_assign_to_typed_ref(zval variable_ptr, zval orig_value, zend_uchar value_type, bool strict)`
	`3565`	`+ZEND_API zval* zend_assign_to_typed_ref_and_result(zval variable_ptr, zval orig_value, zend_uchar value_type, bool strict, zval *result_variable_ptr)`
`3566`	`3566`	`{`
`3567`	`3567`	`bool ret;`
`3568`	`3568`	`zval value;`
`@@ -3582,6 +3582,9 @@ ZEND_API zval* zend_assign_to_typed_ref(zval variable_ptr, zval orig_value, ze`
`3582`	`3582`	`} else {`
`3583`	`3583`	`zval_ptr_dtor_nogc(&value);`
`3584`	`3584`	`}`
	`3585`	`+ if (result_variable_ptr) {`
	`3586`	`+ ZVAL_COPY(result_variable_ptr, variable_ptr);`
	`3587`	`+ }`
`3585`	`3588`	`if (value_type & (IS_VAR\|IS_TMP_VAR)) {`
`3586`	`3589`	`if (UNEXPECTED(ref)) {`
`3587`	`3590`	`if (UNEXPECTED(GC_DELREF(ref) == 0)) {`
`@@ -3595,6 +3598,11 @@ ZEND_API zval* zend_assign_to_typed_ref(zval variable_ptr, zval orig_value, ze`
`3595`	`3598`	`return variable_ptr;`
`3596`	`3599`	`}`
`3597`	`3600`
	`3601`	`+ZEND_API zval* zend_assign_to_typed_ref(zval variable_ptr, zval orig_value, zend_uchar value_type, bool strict)`
	`3602`	`+{`
	`3603`	`+ return zend_assign_to_typed_ref_and_result(variable_ptr, orig_value, value_type, strict, NULL);`
	`3604`	`+}`
	`3605`	`+`
`3598`	`3606`	`ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref(zend_property_info prop_info, zval orig_val, bool strict) {`
`3599`	`3607`	`zval *val = orig_val;`
`3600`	`3608`	`if (Z_ISREF_P(val) && ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(val))) {`