Gate closures over named callables behind an allow_named_closures option

A by-name closure payload (a first-class callable over a named function
or method) lets deepclone_from_array() mint a Closure over any function or
method of that name, with no visibility or staticness restriction and
including internal functions such as system() -- a stronger primitive than
anything unserialize() exposes, and a one-step gadget when paired with an
object whose __wakeup/__unserialize/destructor invokes a stored callback.

Both deepclone_to_array() and deepclone_from_array() gain a third parameter,
bool $allow_named_closures = false, that both ends must enable: with it off
(the default) to_array refuses to encode such a closure, and from_array
rejects any payload carrying a by-name closure marker before instantiating
anything.

So the attribute-cache use case does not regress, first-class callables over
a method of their own declaring class declared in a constant expression (e.g.
#[When(self::isStrict(...))]) are now encoded as a declaration-site reference
(mask LONG 1), like anonymous const-expr closures, instead of by name. They
resolve only to a closure the named class itself declares and round-trip
without the opt-in. References whose declaring class cannot be derived from
the closure (cross-class or global-function callables, inherited methods,
runtime-created callables) keep the by-name form and need the opt-in.

allowed_classes still gates Closure for both forms.

Author: nicolas-grekas

CHANGELOG.md

@@ -9,6 +9,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- `deepclone_to_array()` and `deepclone_from_array()` gained a third
+  parameter, `bool $allow_named_closures = false`, gating the by-name
+  encoding of closures over named callables (first-class callables such as
+  `strlen(...)`, `$obj->method(...)`, `Cls::method(...)`, and
+  `Closure::fromCallable()`). Both ends must enable it: with it off (the
+  default) `deepclone_to_array()` refuses to encode such a closure and
+  `deepclone_from_array()` rejects any payload carrying a by-name closure
+  marker, before instantiating anything. A by-name payload can mint a
+  `Closure` over any function or method of that name, including internal
+  functions like `system()`, so it is restricted to ends that trust each
+  other. See SECURITY.md.
+- On PHP 8.5+, first-class callables over a method of their own declaring
+  class declared in a constant expression (e.g.
+  `#[When(self::isStrict(...))]`) now serialize as a reference to their
+  declaration site (the same code-free, `allowed_classes`-gated payload as
+  anonymous const-expr closures), instead of by name. They round-trip
+  without `$allow_named_closures`, restoring the attribute-cache use case for
+  first-class-callable arguments. References whose declaring class cannot be
+  derived from the closure (cross-class or global-function callables,
+  inherited methods, runtime-created callables) still take the by-name path
+  and require the opt-in.
+
+### Changed
+
+- **BC break.** Closures over named callables no longer serialize or resolve
+  by default; they now require `$allow_named_closures` on both
+  `deepclone_to_array()` and `deepclone_from_array()` (see Added). Code that
+  relied on the previous unconditional by-name behavior must pass the flag.
+  Closures declared in constant expressions are unaffected.
+
 - On PHP 8.4+, `deepclone_from_array()` now creates object nodes whose
   payload slots or replayed `__unserialize` state carry a named-closure or
   const-expr-closure marker as

README.md

@@ -78,15 +78,30 @@ deepclone_hydrate($existingUser, ['name' => 'Bob']);
 ## API
 
 ```php
-function deepclone_to_array(mixed $value, ?array $allowed_classes = null): array;
-function deepclone_from_array(array $data, ?array $allowed_classes = null): mixed;
+function deepclone_to_array(mixed $value, ?array $allowed_classes = null, bool $allow_named_closures = false): array;
+function deepclone_from_array(array $data, ?array $allowed_classes = null, bool $allow_named_closures = false): mixed;
 function deepclone_hydrate(object|string $object_or_class, array $vars = [], int $flags = 0): object;
 ```
 
 `$allowed_classes` restricts which classes may be serialized or deserialized
 (`null` = allow all, `[]` = allow none). Case-insensitive, matching
 `unserialize()`'s `allowed_classes` option.
 
+`$allow_named_closures` controls the by-name encoding of closures over named
+callables (first-class callables such as `strlen(...)`, `$obj->method(...)`
+or `Cls::method(...)`, and `Closure::fromCallable()`). It defaults to
+`false`, and **both ends must enable it**: `deepclone_to_array()` refuses to
+encode such a closure unless it is set, and `deepclone_from_array()` refuses
+to resolve a by-name closure payload unless it is set. The reason is that a
+by-name payload can mint a `Closure` over *any* function or method of that
+name, including internal functions like `system()`, so it should only travel
+between ends that trust each other. Closures declared in constant
+expressions (anonymous static closures and first-class callables over a
+method of their own declaring class, e.g. `#[When(self::isStrict(...))]`)
+are **not** affected: they serialize as a reference to their declaration
+site, resolvable only to what the named class itself declares, and round-trip
+without this option.
+
 ### Lazy hydration of closure-bearing nodes (PHP 8.4+)
 
 `deepclone_from_array()` creates the object nodes that are expensive to
@@ -257,7 +272,12 @@ $s->__unserialize([[$obj1, 'info1', $obj2, 'info2'], []]);
 - Cycles in the object graph
 - Private/protected properties across inheritance
 - `__serialize` / `__unserialize` / `__sleep` / `__wakeup` semantics
-- Named closures (first-class callables like `strlen(...)`)
+- Closures declared in constant expressions (anonymous static closures and
+  first-class callables over a method of their declaring class, as found in
+  attribute arguments and parameter defaults), as a reference to their
+  declaration site
+- Closures over named callables (first-class callables like `strlen(...)`),
+  by name, when `$allow_named_closures` is enabled on both ends
 - Enum values
 - Copy-on-write for strings and scalar arrays
 

SECURITY.md

@@ -30,3 +30,25 @@ and throws `\ValueError` on malformed input. It instantiates classes named in
 the payload via the standard PHP class loader. Treat its input the same way
 you would treat input to `unserialize()`: only call it on payloads from
 trusted sources.
+
+### Closures
+
+A by-name closure payload (a first-class callable over a named function or
+method, e.g. `strlen(...)` or `Cls::method(...)`) is a stronger primitive
+than anything `unserialize()` exposes: resolving it mints a `Closure` over
+the named callable, with no visibility or staticness restriction, including
+internal functions such as `system()`. Creating the closure does not call it,
+but a payload pairing such a closure with an object whose `__wakeup`,
+`__unserialize` or destructor invokes a stored callback turns it into a
+one-step gadget. This encoding is therefore **off by default** and gated by
+the `$allow_named_closures` flag, which both `deepclone_to_array()` and
+`deepclone_from_array()` must set: a default `deepclone_from_array()` call
+rejects any payload carrying a by-name closure before instantiating anything.
+Enable it only between ends that trust each other.
+
+Closures declared in constant expressions (the attribute-cache use case)
+carry no such risk and need no opt-in: they serialize as a reference to their
+declaration site and resolve only to a closure the named class itself
+declares, the same bounded capability as unserializing a class or enum-case
+name. The `$allowed_classes` filter still applies to both forms: omit
+`Closure` from the list and no closure resolves at all.

deepclone.c

@@ -265,6 +265,7 @@ struct _dc_ctx {
 	uint32_t       next_obj_id;
 	uint32_t       objects_count;
 	bool           is_static;
+	bool           allow_named_closures; /* opt-in: encode closures over named callables by name */
 	HashTable     *allowed_ht;     /* allowed class names set (or NULL = all) */
 
 	/* Output structures built incrementally during traversal */
@@ -311,6 +312,7 @@ static void dc_ctx_init(dc_ctx *ctx) {
 	ctx->next_obj_id = 0;
 	ctx->objects_count = 0;
 	ctx->is_static = 1;
+	ctx->allow_named_closures = false;
 	ctx->allowed_ht = NULL;
 	zend_hash_init(&ctx->scope_cache, 4, NULL, ZVAL_PTR_DTOR, 0);
 	zend_hash_init(&ctx->class_info, 4, NULL, NULL, 0);
@@ -1782,10 +1784,54 @@ static void dc_copy_value(dc_ctx *ctx, zval *src, zval *dst, zval *mask_dst)
 		}
 	}
 
-	/* ── Named closure ──────────────────────────── */
+	/* ── Closures ───────────────────────────────── */
 	if (Z_OBJCE_P(src) == zend_ce_closure) {
 		const zend_function *func = zend_get_closure_method_def(Z_OBJ_P(src));
+
+#if PHP_VERSION_ID >= 80500
+		/* Const-expr declaration-site reference. This covers anonymous static
+		 * closures and first-class callables over a method of their own
+		 * declaring class (e.g. #[When(self::isStrict(...))]). It is attempted
+		 * before the by-name encoding so that such closures serialize as a
+		 * declaration-site reference — resolvable only to what the class
+		 * itself declares — and therefore round-trip without requiring the
+		 * allow_named_closures opt-in. The allow-list is checked first so that
+		 * disallowing Closure is reported before any const-expr of the scope
+		 * class is evaluated. */
+		if (func && func->type == ZEND_USER_FUNCTION && func->common.scope) {
+			zval *this_ptr = zend_get_closure_this_ptr(src);
+			if (!this_ptr || Z_TYPE_P(this_ptr) != IS_OBJECT) {
+				if (!dc_class_allowed(ctx->allowed_ht, zend_ce_closure->name)) {
+					zend_value_error("deepclone_to_array(): class \"Closure\" is not allowed");
+					return;
+				}
+				zval payload;
+				ZVAL_UNDEF(&payload);
+				if (dc_cexpr_locate(func, &payload)) {
+					ZVAL_COPY_VALUE(dst, &payload);
+					DC_MASK_CONSTEXPR_CLOSURE(mask_dst);
+					goto handle_value;
+				}
+				if (UNEXPECTED(EG(exception))) {
+					return;
+				}
+			}
+		}
+#endif
+
+		/* Named closure: a first-class callable that is not addressable as a
+		 * declaration-site reference (one created at runtime, or whose target
+		 * lives outside its declaring class, or over an internal/global
+		 * function). Encoding it stores the callable by name, which lets
+		 * deepclone_from_array() mint a Closure over any function or method of
+		 * that name — including internal functions such as system(). It is
+		 * therefore gated behind the allow_named_closures opt-in, which both
+		 * ends must enable. */
 		if (func && (func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE)) {
+			if (!ctx->allow_named_closures) {
+				zend_value_error("deepclone_to_array(): serializing a closure over the named callable \"%s\" requires enabling the allow_named_closures option", ZSTR_VAL(func->common.function_name));
+				return;
+			}
 			if (!dc_class_allowed(ctx->allowed_ht, zend_ce_closure->name)) {
 				zend_value_error("deepclone_to_array(): class \"Closure\" is not allowed");
 				return;
@@ -1846,32 +1892,8 @@ static void dc_copy_value(dc_ctx *ctx, zval *src, zval *dst, zval *mask_dst)
 			DC_MASK_NAMED_CLOSURE(mask_dst);
 			goto handle_value;
 		}
-
-#if PHP_VERSION_ID >= 80500
-		/* Anonymous closure: reference its const-expr declaration site.
-		 * The allow-list is checked first so that disallowing Closure is
-		 * reported before any const-expr of the scope class is evaluated. */
-		if (func && func->type == ZEND_USER_FUNCTION && func->common.scope) {
-			zval *this_ptr = zend_get_closure_this_ptr(src);
-			if (!this_ptr || Z_TYPE_P(this_ptr) != IS_OBJECT) {
-				if (!dc_class_allowed(ctx->allowed_ht, zend_ce_closure->name)) {
-					zend_value_error("deepclone_to_array(): class \"Closure\" is not allowed");
-					return;
-				}
-				zval payload;
-				ZVAL_UNDEF(&payload);
-				if (dc_cexpr_locate(func, &payload)) {
-					ZVAL_COPY_VALUE(dst, &payload);
-					DC_MASK_CONSTEXPR_CLOSURE(mask_dst);
-					goto handle_value;
-				}
-				if (UNEXPECTED(EG(exception))) {
-					return;
-				}
-			}
-		}
-#endif
-		/* Other anonymous closure — fall through to regular object handling */
+		/* Other closure (runtime anonymous, arrow fn) — fall through to
+		 * regular object handling, which refuses it as non-instantiable. */
 	}
 
 	/* ── Regular object processing ──────────────── */
@@ -2722,11 +2744,13 @@ PHP_FUNCTION(deepclone_to_array)
 {
 	zval *value;
 	HashTable *allowed_ht = NULL;
+	bool allow_named_closures = false;
 
-	ZEND_PARSE_PARAMETERS_START(1, 2)
+	ZEND_PARSE_PARAMETERS_START(1, 3)
 		Z_PARAM_ZVAL(value)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_ARRAY_HT_OR_NULL(allowed_ht)
+		Z_PARAM_BOOL(allow_named_closures)
 	ZEND_PARSE_PARAMETERS_END();
 
 	/* Reject resources at the top level just like the walker does mid-tree.
@@ -2755,6 +2779,7 @@ PHP_FUNCTION(deepclone_to_array)
 
 	dc_ctx ctx;
 	dc_ctx_init(&ctx);
+	ctx.allow_named_closures = allow_named_closures;
 	if (allowed_ht) {
 		ctx.allowed_ht = dc_build_allowed_set(allowed_ht, "deepclone_to_array");
 		if (!ctx.allowed_ht) {
@@ -3767,11 +3792,77 @@ static bool dc_mask_has_closure(zval *mask)
 	return false;
 }
 
+/* Like dc_mask_has_closure() but matches only the named-closure marker
+ * (LONG(0)), ignoring const-expr-closure references (LONG(1)). */
+static bool dc_mask_has_named_closure(zval *mask)
+{
+	if (mask == NULL) {
+		return false;
+	}
+	if (DC_MASK_IS_NAMED_CLOSURE(mask)) {
+		return true;
+	}
+	if (Z_TYPE_P(mask) != IS_ARRAY) {
+		return false;
+	}
+	zval *v;
+	ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(mask), v) {
+		if (dc_mask_has_named_closure(v)) {
+			return true;
+		}
+	} ZEND_HASH_FOREACH_END();
+	return false;
+}
+
+/* Scan the four payload regions that can carry closure markers — the top
+ * mask, the resolve table, the reference masks and the replayed state masks —
+ * for a named-closure marker. Mirrors the region set used by the
+ * allowed_classes "Closure" gate below. */
+static bool dc_payload_has_named_closure(zval *zmask, zval *zresolve, zval *zref_masks, zval *zstates)
+{
+	if (dc_mask_has_named_closure(zmask)) {
+		return true;
+	}
+	if (zresolve) {
+		zval *scope;
+		ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(zresolve), scope) {
+			if (Z_TYPE_P(scope) != IS_ARRAY) continue;
+			zval *name;
+			ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(scope), name) {
+				if (dc_mask_has_named_closure(name)) {
+					return true;
+				}
+			} ZEND_HASH_FOREACH_END();
+		} ZEND_HASH_FOREACH_END();
+	}
+	if (zref_masks) {
+		zval *rmask;
+		ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(zref_masks), rmask) {
+			if (dc_mask_has_named_closure(rmask)) {
+				return true;
+			}
+		} ZEND_HASH_FOREACH_END();
+	}
+	if (zstates) {
+		zval *state;
+		ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(zstates), state) {
+			if (Z_TYPE_P(state) == IS_ARRAY) {
+				zval *smask = zend_hash_index_find(Z_ARRVAL_P(state), 2);
+				if (smask && dc_mask_has_named_closure(smask)) {
+					return true;
+				}
+			}
+		} ZEND_HASH_FOREACH_END();
+	}
+	return false;
+}
+
 PHP_FUNCTION(deepclone_from_array)
 {
 	HashTable *data_ht;
 	HashTable *allowed_ht = NULL;
 	HashTable *allowed_set = NULL;
+	bool allow_named_closures = false;
 	HashTable refs_local;
 	HashTable *refs = NULL;
 	zend_string **class_names = NULL;
@@ -3795,10 +3886,11 @@ PHP_FUNCTION(deepclone_from_array)
 	 * any early-exit path. Only one is live at a time. */
 	zend_string *numeric_prop_tmp = NULL;
 
-	ZEND_PARSE_PARAMETERS_START(1, 2)
+	ZEND_PARSE_PARAMETERS_START(1, 3)
 		Z_PARAM_ARRAY_HT(data_ht)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_ARRAY_HT_OR_NULL(allowed_ht)
+		Z_PARAM_BOOL(allow_named_closures)
 	ZEND_PARSE_PARAMETERS_END();
 
 	/* Static value: return data['value'] */
@@ -3914,6 +4006,18 @@ PHP_FUNCTION(deepclone_from_array)
 		}
 	}
 
+	/* Named closures (the by-name marker, LONG(0)) let a payload mint a
+	 * Closure over any function or method by name; they resolve only when the
+	 * caller opts in via allow_named_closures, which the producer must also
+	 * have set. Const-expr-closure references (LONG(1)) are unaffected: they
+	 * resolve only to closures the named class itself declares. The scan runs
+	 * before any object is instantiated, so a payload carrying a named closure
+	 * is rejected wholesale rather than failing mid-hydration. */
+	if (!allow_named_closures
+			&& dc_payload_has_named_closure(zmask, zresolve, zref_masks, zstates)) {
+		DC_INVALID("deepclone_from_array(): resolving a closure over a named callable requires enabling the allow_named_closures option");
+	}
+
 	/* ── Build objectMeta ── */
 	if (Z_TYPE_P(zobject_meta) == IS_LONG) {
 		zend_long n = Z_LVAL_P(zobject_meta);

deepclone.stub.php

@@ -47,9 +47,9 @@ private function hydrate(object $object): void {}
      */
     const DEEPCLONE_HYDRATE_PRESERVE_REFS = UNKNOWN;
 
-    function deepclone_to_array(mixed $value, ?array $allowed_classes = null): array {}
+    function deepclone_to_array(mixed $value, ?array $allowed_classes = null, bool $allow_named_closures = false): array {}
 
-    function deepclone_from_array(array $data, ?array $allowed_classes = null): mixed {}
+    function deepclone_from_array(array $data, ?array $allowed_classes = null, bool $allow_named_closures = false): mixed {}
 
     function deepclone_hydrate(object|string $object_or_class, array $vars = [], int $flags = 0): object {}
 }

deepclone_arginfo.h

@@ -1,14 +1,16 @@
 /* This is a generated file, edit deepclone.stub.php instead.
- * Stub hash: bde61513c175dd9130f054c427cfaad2e233ff4f */
+ * Stub hash: ec11b6f3b9b69f77cbddece18176eab2ffd1007a */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_deepclone_to_array, 0, 1, IS_ARRAY, 0)
 	ZEND_ARG_TYPE_INFO(0, value, IS_MIXED, 0)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, allowed_classes, IS_ARRAY, 1, "null")
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, allow_named_closures, _IS_BOOL, 0, "false")
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_deepclone_from_array, 0, 1, IS_MIXED, 0)
 	ZEND_ARG_TYPE_INFO(0, data, IS_ARRAY, 0)
 	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, allowed_classes, IS_ARRAY, 1, "null")
+	ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, allow_named_closures, _IS_BOOL, 0, "false")
 ZEND_END_ARG_INFO()
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_deepclone_hydrate, 0, 1, IS_OBJECT, 0)