Skip to content

Commit 4fcf0db

Browse files
committed
Fix use after free when rebinding __call closure
We would end up freeing the function name twice here, once for the original closure, and once for the rebound one. Rather than further special casing the zend_closure_call_magic case, always addref the function_name for internal functions, the same we do for userland functions. To compensate, we need to release the original function name when creating from callable or call frame. Fixes oss-fuzz #37695.
1 parent fdc6082 commit 4fcf0db

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

Zend/tests/closure_call_bind.phpt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
Calling bindTo() on __call() closure
3+
--FILE--
4+
<?php
5+
6+
class Foo {
7+
function __call($name, $args) {
8+
echo "__call($name)\n";
9+
}
10+
}
11+
12+
$foo = new Foo;
13+
$name = "foo";
14+
Closure::fromCallable([$foo, $name . "bar"])->bindTo(new Foo)();
15+
$foo->{$name . "bar"}(...)->bindTo(new Foo)();
16+
17+
?>
18+
--EXPECT--
19+
__call(foobar)
20+
__call(foobar)

Zend/zend_closures.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,10 @@ static zend_result zend_create_closure_from_callable(zval *return_value, zval *c
341341
zend_create_fake_closure(return_value, mptr, mptr->common.scope, fcc.called_scope, NULL);
342342
}
343343

344+
if (&mptr->internal_function == &call) {
345+
zend_string_release(mptr->common.function_name);
346+
}
347+
344348
return SUCCESS;
345349
}
346350
/* }}} */
@@ -482,7 +486,7 @@ static void zend_closure_free_storage(zend_object *object) /* {{{ */
482486
zend_destroy_static_vars(&closure->func.op_array);
483487
}
484488
destroy_op_array(&closure->func.op_array);
485-
} else if (closure->orig_internal_handler == zend_closure_call_magic) {
489+
} else if (closure->func.type == ZEND_INTERNAL_FUNCTION) {
486490
zend_string_release(closure->func.common.function_name);
487491
}
488492

@@ -739,6 +743,7 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
739743
closure->orig_internal_handler = closure->func.internal_function.handler;
740744
}
741745
closure->func.internal_function.handler = zend_closure_internal_handler;
746+
zend_string_addref(closure->func.op_array.function_name);
742747
if (!func->common.scope) {
743748
/* if it's a free function, we won't set scope & this since they're meaningless */
744749
this_ptr = NULL;
@@ -811,6 +816,10 @@ void zend_closure_from_frame(zval *return_value, zend_execute_data *call) { /* {
811816
} else {
812817
zend_create_fake_closure(return_value, mptr, mptr->common.scope, Z_CE(call->This), NULL);
813818
}
819+
820+
if (&mptr->internal_function == &trampoline) {
821+
zend_string_release(mptr->common.function_name);
822+
}
814823
} /* }}} */
815824

816825
void zend_closure_bind_var(zval *closure_zv, zend_string *var_name, zval *var) /* {{{ */

0 commit comments

Comments
 (0)