Correctly report failure in zend_handle_undef_args()

And do the check before increfing the closure object, otherwise we'd have to release it as well. Fixes oss-fuzz #25313.
php · Aug 31, 2020 · 061c708 · 061c708
1 parent e81becc
commit 061c708
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 11 deletions.
diff --git a/Zend/tests/named_params/call_user_func.phpt b/Zend/tests/named_params/call_user_func.phpt
@@ -12,6 +12,9 @@ $test_variadic = function(...$args) {
 $test_ref = function(&$ref) {
     $ref++;
 };
+$test_required = function($a, $b) {
+    echo "a = $a, b = $b\n";
+};
 
 class Test {
     public function __construct($a = 'a', $b = 'b', $c = 'c') {
@@ -31,6 +34,11 @@ call_user_func($test, c: 'C');
 call_user_func($test_variadic, 'A', c: 'C');
 call_user_func($test_ref, ref: null);
 var_dump(call_user_func('call_user_func', $test, c: 'D'));
+try {
+    call_user_func($test_required, b: 'B');
+} catch (ArgumentCountError $e) {
+    echo $e->getMessage(), "\n";
+}
 try {
     var_dump(call_user_func('array_slice', [1, 2, 3, 4, 5], length: 2));
 } catch (ArgumentCountError $e) {
@@ -74,6 +82,7 @@ array(2) {
 Warning: {closure}(): Argument #1 ($ref) must be passed by reference, value given in %s on line %d
 a = a, b = b, c = D
 NULL
+{closure}(): Argument #1 ($a) not passed
 array_slice(): Argument #2 ($offset) not passed
 array(2) {
   [3]=>

diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -4491,6 +4491,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_handle_undef_args(zend_execute_data *cal
 				start_fake_frame(call, opline);
 				zend_argument_error(zend_ce_argument_count_error, i + 1, "not passed");
 				end_fake_frame(call);
+				return FAILURE;
 			}
 		}
 

diff --git a/Zend/zend_execute_API.c b/Zend/zend_execute_API.c
@@ -839,17 +839,6 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_
 		} ZEND_HASH_FOREACH_END();
 	}
 
-	if (UNEXPECTED(func->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
-		uint32_t call_info;
-
-		GC_ADDREF(ZEND_CLOSURE_OBJECT(func));
-		call_info = ZEND_CALL_CLOSURE;
-		if (func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE) {
-			call_info |= ZEND_CALL_FAKE_CLOSURE;
-		}
-		ZEND_ADD_CALL_FLAG(call, call_info);
-	}
-
 	if (UNEXPECTED(ZEND_CALL_INFO(call) & ZEND_CALL_MAY_HAVE_UNDEF)) {
 		if (zend_handle_undef_args(call) == FAILURE) {
 			zend_vm_stack_free_args(call);
@@ -861,6 +850,17 @@ zend_result zend_call_function(zend_fcall_info *fci, zend_fcall_info_cache *fci_
 		}
 	}
 
+	if (UNEXPECTED(func->op_array.fn_flags & ZEND_ACC_CLOSURE)) {
+		uint32_t call_info;
+
+		GC_ADDREF(ZEND_CLOSURE_OBJECT(func));
+		call_info = ZEND_CALL_CLOSURE;
+		if (func->common.fn_flags & ZEND_ACC_FAKE_CLOSURE) {
+			call_info |= ZEND_CALL_FAKE_CLOSURE;
+		}
+		ZEND_ADD_CALL_FLAG(call, call_info);
+	}
+
 	orig_fake_scope = EG(fake_scope);
 	EG(fake_scope) = NULL;
 	if (func->type == ZEND_USER_FUNCTION) {