Fix phpGH-17442: Engine UAF with reference assign and dtor

nielsdos · Jan 26, 2025 · 6852055 · 6852055
1 parent 75d7684
commit 6852055
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 22 deletions.
diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS
@@ -14,6 +14,11 @@ PHP 8.5 INTERNALS UPGRADE NOTES
 1. Internal API changes
 ========================
 
+- Zend
+  . Added ZEND_SAFE_ASSIGN_VALUE() macro to safely assign a value to a zval.
+  . Added zval_ptr_safe_dtor() to safely destroy a zval when a destructor
+    could interfere.
+
 ========================
 2. Build system changes
 ========================

diff --git a/Zend/zend_API.c b/Zend/zend_API.c
@@ -4666,8 +4666,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_ex(zend_reference *ref, zval *val
 		zval_ptr_dtor(val);
 		return FAILURE;
 	} else {
-		zval_ptr_dtor(&ref->val);
-		ZVAL_COPY_VALUE(&ref->val, val);
+		ZEND_SAFE_ASSIGN_VALUE(&ref->val, val);
 		return SUCCESS;
 	}
 }

diff --git a/Zend/zend_API.h b/Zend/zend_API.h
@@ -1097,6 +1097,22 @@ ZEND_API zend_result zend_try_assign_typed_ref_res(zend_reference *ref, zend_res
 ZEND_API zend_result zend_try_assign_typed_ref_zval(zend_reference *ref, zval *zv);
 ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval *zv, bool strict);
 
+#define ZEND_SAFE_ASSIGN_VALUE(zv, value) do { \
+	zval *_zv = zv; \
+	zval *_value = value; \
+	if (Z_REFCOUNTED_P(_zv)) { \
+		zend_refcounted *rc = Z_COUNTED_P(_zv); \
+		ZVAL_COPY_VALUE(_zv, _value); \
+		if (!GC_DELREF(rc)) { \
+			rc_dtor_func(rc); \
+		} else { \
+			gc_check_possible_root(rc); \
+		} \
+	} else { \
+		ZVAL_COPY_VALUE(_zv, _value); \
+	} \
+} while (0)
+
 #define _ZEND_TRY_ASSIGN_NULL(zv, is_ref) do { \
 	zval *_zv = zv; \
 	if (is_ref || UNEXPECTED(Z_ISREF_P(_zv))) { \
@@ -1107,7 +1123,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_NULL(_zv); \
 } while (0)
 
@@ -1129,7 +1145,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_FALSE(_zv); \
 } while (0)
 
@@ -1151,7 +1167,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_TRUE(_zv); \
 } while (0)
 
@@ -1173,7 +1189,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_BOOL(_zv, bval); \
 } while (0)
 
@@ -1195,7 +1211,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_LONG(_zv, lval); \
 } while (0)
 
@@ -1217,7 +1233,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_DOUBLE(_zv, dval); \
 } while (0)
 
@@ -1239,7 +1255,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_EMPTY_STRING(_zv); \
 } while (0)
 
@@ -1261,7 +1277,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_STR(_zv, str); \
 } while (0)
 
@@ -1283,7 +1299,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_NEW_STR(_zv, str); \
 } while (0)
 
@@ -1305,7 +1321,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_STRING(_zv, string); \
 } while (0)
 
@@ -1327,7 +1343,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_STRINGL(_zv, string, len); \
 } while (0)
 
@@ -1349,7 +1365,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_ARR(_zv, arr); \
 } while (0)
 
@@ -1371,7 +1387,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_RES(_zv, res); \
 } while (0)
 
@@ -1393,7 +1409,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_COPY_VALUE(_zv, other_zv); \
 } while (0)
 
@@ -1415,7 +1431,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_COPY_VALUE(_zv, other_zv); \
 } while (0)
 
@@ -1447,7 +1463,7 @@ ZEND_API zend_result zend_try_assign_typed_ref_zval_ex(zend_reference *ref, zval
 		} \
 		_zv = &ref->val; \
 	} \
-	zval_ptr_dtor(_zv); \
+	zval_ptr_safe_dtor(_zv); \
 	ZVAL_COPY_VALUE(_zv, other_zv); \
 } while (0)
 
@@ -1485,10 +1501,7 @@ static zend_always_inline zval *zend_try_array_init_size(zval *zv, uint32_t size
 		}
 		zv = &ref->val;
 	}
-	zval garbage;
-	ZVAL_COPY_VALUE(&garbage, zv);
-	ZVAL_NULL(zv);
-	zval_ptr_dtor(&garbage);
+	zval_ptr_safe_dtor(zv);
 	ZVAL_ARR(zv, arr);
 	return zv;
 }

diff --git a/Zend/zend_variables.c b/Zend/zend_variables.c
@@ -85,6 +85,20 @@ ZEND_API void zval_ptr_dtor(zval *zval_ptr) /* {{{ */
 }
 /* }}} */
 
+ZEND_API void zval_ptr_safe_dtor(zval *zval_ptr)
+{
+	if (Z_REFCOUNTED_P(zval_ptr)) {
+		zend_refcounted *ref = Z_COUNTED_P(zval_ptr);
+
+		if (GC_DELREF(ref) == 0) {
+			ZVAL_NULL(zval_ptr);
+			rc_dtor_func(ref);
+		} else {
+			gc_check_possible_root(ref);
+		}
+	}
+}
+
 ZEND_API void zval_internal_ptr_dtor(zval *zval_ptr) /* {{{ */
 {
 	if (Z_REFCOUNTED_P(zval_ptr)) {

diff --git a/Zend/zend_variables.h b/Zend/zend_variables.h
@@ -78,6 +78,7 @@ static zend_always_inline void zval_ptr_dtor_str(zval *zval_ptr)
 }
 
 ZEND_API void zval_ptr_dtor(zval *zval_ptr);
+ZEND_API void zval_ptr_safe_dtor(zval *zval_ptr);
 ZEND_API void zval_internal_ptr_dtor(zval *zvalue);
 
 /* Kept for compatibility */