Skip to content

Dont check arginfo types for internal functions #4232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

Dont check arginfo types for internal functions #4232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
63f0ab4
Allow null for weak scalar arg of internal func in arginfo
nikic Jun 6, 2019
db3d5fd
Don't verify arginfo types for internal functions
nikic Jun 6, 2019
2b65702
Update tests for arginfo->zpp error message change
nikic Jun 6, 2019
ae50ec3
ctor_params of mysqli_fetch_object should use "a"
nikic Jun 6, 2019
5a59736
Remove bogus quieting in AppendIterator::append()
nikic Jun 6, 2019
34c6b54
Add some ATTRIBUTE_UNUSED
nikic Jun 6, 2019
b6271f9
Move arginfo check before function call
nikic Jun 14, 2019
f39e337
Don't perform type conversions during arginfo sanity check
nikic Jun 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 63 additions & 24 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,35 @@ static zend_bool zend_verify_weak_scalar_type_hint(zend_uchar type_hint, zval *a
}
}

static zend_bool zend_verify_scalar_type_hint(zend_uchar type_hint, zval *arg, zend_bool strict)
#if ZEND_DEBUG
/* Used to sanity-check internal arginfo types without performing any actual type conversions. */
static zend_bool zend_verify_weak_scalar_type_hint_no_sideeffect(zend_uchar type_hint, zval *arg)
{
switch (type_hint) {
case _IS_BOOL: {
zend_bool dest;
return zend_parse_arg_bool_weak(arg, &dest);
}
case IS_LONG: {
zend_long dest;
return zend_parse_arg_long_weak(arg, &dest);
}
case IS_DOUBLE: {
double dest;
return zend_parse_arg_double_weak(arg, &dest);
}
case IS_STRING:
/* We don't call cast_object here, because this check must be side-effect free. As this
* is only used for a sanity check of arginfo/zpp consistency, it's okay if we accept
* more than actually allowed here. */
return Z_TYPE_P(arg) < IS_STRING || Z_TYPE_P(arg) == IS_OBJECT;
default:
return 0;
}
}
#endif

static zend_bool zend_verify_scalar_type_hint(zend_uchar type_hint, zval *arg, zend_bool strict, zend_bool is_internal_arg)
{
if (UNEXPECTED(strict)) {
/* SSTH Exception: IS_LONG may be accepted as IS_DOUBLE (converted) */
Expand All @@ -897,8 +925,17 @@ static zend_bool zend_verify_scalar_type_hint(zend_uchar type_hint, zval *arg, z
}
} else if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL)) {
/* NULL may be accepted only by nullable hints (this is already checked) */
if (is_internal_arg && (type_hint <= IS_STRING || type_hint == _IS_BOOL)) {
/* As an exception, null is allowed for scalar types in weak mode. */
return 1;
}
return 0;
}
#if ZEND_DEBUG
if (is_internal_arg) {
return zend_verify_weak_scalar_type_hint_no_sideeffect(type_hint, arg);
}
#endif
return zend_verify_weak_scalar_type_hint(type_hint, arg);
}

Expand Down Expand Up @@ -986,7 +1023,7 @@ static zend_always_inline zend_bool i_zend_check_property_type(zend_property_inf
} else if (ZEND_TYPE_CODE(info->type) == IS_ITERABLE) {
return zend_is_iterable(property);
} else {
return zend_verify_scalar_type_hint(ZEND_TYPE_CODE(info->type), property, strict);
return zend_verify_scalar_type_hint(ZEND_TYPE_CODE(info->type), property, strict, 0);
}
}

Expand Down Expand Up @@ -1024,7 +1061,7 @@ static zend_always_inline zend_bool zend_check_type(
zend_type type,
zval *arg, zend_class_entry **ce, void **cache_slot,
zval *default_value, zend_class_entry *scope,
zend_bool is_return_type)
zend_bool is_return_type, zend_bool is_internal_arg)
{
zend_reference *ref = NULL;

Expand Down Expand Up @@ -1071,7 +1108,8 @@ static zend_always_inline zend_bool zend_check_type(
return 0; /* we cannot have conversions for typed refs */
} else {
return zend_verify_scalar_type_hint(ZEND_TYPE_CODE(type), arg,
is_return_type ? ZEND_RET_USES_STRICT_TYPES() : ZEND_ARG_USES_STRICT_TYPES());
is_return_type ? ZEND_RET_USES_STRICT_TYPES() : ZEND_ARG_USES_STRICT_TYPES(),
is_internal_arg);
}

/* Special handling for IS_VOID is not necessary (for return types),
Expand All @@ -1092,7 +1130,7 @@ static zend_always_inline int zend_verify_arg_type(zend_function *zf, uint32_t a
}

ce = NULL;
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0))) {
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0, 0))) {
zend_verify_arg_error(zf, cur_arg_info, arg_num, ce, arg);
return 0;
}
Expand All @@ -1109,7 +1147,7 @@ static zend_always_inline int zend_verify_recv_arg_type(zend_function *zf, uint3
cur_arg_info = &zf->common.arg_info[arg_num-1];

ce = NULL;
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0))) {
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0, 0))) {
zend_verify_arg_error(zf, cur_arg_info, arg_num, ce, arg);
return 0;
}
Expand All @@ -1127,29 +1165,37 @@ static zend_always_inline int zend_verify_variadic_arg_type(zend_function *zf, u
cur_arg_info = &zf->common.arg_info[zf->common.num_args];

ce = NULL;
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0))) {
if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, cache_slot, default_value, zf->common.scope, 0, 0))) {
zend_verify_arg_error(zf, cur_arg_info, arg_num, ce, arg);
return 0;
}

return 1;
}

static zend_never_inline int zend_verify_internal_arg_types(zend_function *fbc, zend_execute_data *call)
static zend_never_inline ZEND_ATTRIBUTE_UNUSED int zend_verify_internal_arg_types(zend_function *fbc, zend_execute_data *call)
{
uint32_t i;
uint32_t num_args = ZEND_CALL_NUM_ARGS(call);
zval *p = ZEND_CALL_ARG(call, 1);
void *dummy_cache_slot;
zval *arg = ZEND_CALL_ARG(call, 1);

for (i = 0; i < num_args; ++i) {
dummy_cache_slot = NULL;
if (UNEXPECTED(!zend_verify_arg_type(fbc, i + 1, p, NULL, &dummy_cache_slot))) {
EG(current_execute_data) = call->prev_execute_data;
zend_vm_stack_free_args(call);
zend_arg_info *cur_arg_info;
zend_class_entry *ce = NULL;
void *dummy_cache_slot = NULL;

if (EXPECTED(i < fbc->common.num_args)) {
cur_arg_info = &fbc->common.arg_info[i];
} else if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_VARIADIC)) {
cur_arg_info = &fbc->common.arg_info[fbc->common.num_args];
} else {
break;
}

if (UNEXPECTED(!zend_check_type(cur_arg_info->type, arg, &ce, &dummy_cache_slot, NULL, fbc->common.scope, 0, /* is_internal_arg */ 1))) {
return 0;
}
p++;
arg++;
}
return 1;
}
Expand Down Expand Up @@ -1242,7 +1288,7 @@ static int zend_verify_internal_return_type(zend_function *zf, zval *ret)
return 1;
}

if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, &dummy_cache_slot, NULL, NULL, 1))) {
if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, &dummy_cache_slot, NULL, NULL, 1, 0))) {
zend_verify_internal_return_error(zf, ce, ret);
return 0;
}
Expand All @@ -1256,7 +1302,7 @@ static zend_always_inline void zend_verify_return_type(zend_function *zf, zval *
zend_arg_info *ret_info = zf->common.arg_info - 1;
zend_class_entry *ce = NULL;

if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, cache_slot, NULL, NULL, 1))) {
if (UNEXPECTED(!zend_check_type(ret_info->type, ret, &ce, cache_slot, NULL, NULL, 1, 0))) {
zend_verify_return_error(zf, ce, ret);
}
}
Expand Down Expand Up @@ -4439,13 +4485,6 @@ ZEND_API zval *zend_get_zval_ptr(const zend_op *opline, int op_type, const znode
return ret;
}

ZEND_API void ZEND_FASTCALL zend_check_internal_arg_type(zend_function *zf, uint32_t arg_num, zval *arg)
{
void *dummy_cache_slot = NULL;

zend_verify_arg_type(zf, arg_num, arg, NULL, &dummy_cache_slot);
}

ZEND_API int ZEND_FASTCALL zend_check_arg_type(zend_function *zf, uint32_t arg_num, zval *arg, zval *default_value, void **cache_slot)
{
return zend_verify_arg_type(zf, arg_num, arg, default_value, cache_slot);
Expand Down
1 change: 0 additions & 1 deletion Zend/zend_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ ZEND_API int zend_eval_stringl_ex(const char *str, size_t str_len, zval *retval_
/* export zend_pass_function to allow comparisons against it */
extern ZEND_API const zend_internal_function zend_pass_function;

ZEND_API void ZEND_FASTCALL zend_check_internal_arg_type(zend_function *zf, uint32_t arg_num, zval *arg);
ZEND_API int ZEND_FASTCALL zend_check_arg_type(zend_function *zf, uint32_t arg_num, zval *arg, zval *default_value, void **cache_slot);
ZEND_API ZEND_COLD void ZEND_FASTCALL zend_missing_arg_error(zend_execute_data *execute_data);

Expand Down
52 changes: 30 additions & 22 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3954,13 +3954,15 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL))
call->prev_execute_data = execute_data;
EG(current_execute_data) = call;

if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS)
&& UNEXPECTED(!zend_verify_internal_arg_types(fbc, call))) {
zend_vm_stack_free_call_frame(call);
zend_rethrow_exception(execute_data);
UNDEF_RESULT();
HANDLE_EXCEPTION();
}

#if ZEND_DEBUG
/* Type checks for internal functions are usually only performed by zpp.
* In debug mode we additionally run arginfo checks to detect cases where
* arginfo and zpp went out of sync. */
zend_bool wrong_arg_types =
(fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) &&
!zend_verify_internal_arg_types(fbc, call);
#endif

ret = RETURN_VALUE_USED(opline) ? EX_VAR(opline->result.var) : &retval;
ZVAL_NULL(ret);
Expand All @@ -3969,6 +3971,7 @@ ZEND_VM_HOT_HANDLER(131, ZEND_DO_FCALL_BY_NAME, ANY, ANY, SPEC(RETVAL))

#if ZEND_DEBUG
if (!EG(exception) && call->func) {
ZEND_ASSERT(!wrong_arg_types && "Arginfo / zpp type mismatch?");
ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
zend_verify_internal_return_type(call->func, ret));
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
Expand Down Expand Up @@ -4040,11 +4043,14 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL))
call->prev_execute_data = execute_data;
EG(current_execute_data) = call;

if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS)
&& UNEXPECTED(!zend_verify_internal_arg_types(fbc, call))) {
UNDEF_RESULT();
ZEND_VM_C_GOTO(fcall_end);
}
#if ZEND_DEBUG
/* Type checks for internal functions are usually only performed by zpp.
* In debug mode we additionally run arginfo checks to detect cases where
* arginfo and zpp went out of sync. */
zend_bool wrong_arg_types =
(fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) &&
!zend_verify_internal_arg_types(fbc, call);
#endif

ret = RETURN_VALUE_USED(opline) ? EX_VAR(opline->result.var) : &retval;
ZVAL_NULL(ret);
Expand All @@ -4058,6 +4064,7 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL))

#if ZEND_DEBUG
if (!EG(exception) && call->func) {
ZEND_ASSERT(!wrong_arg_types && "Arginfo / zpp type mismatch?");
ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
zend_verify_internal_return_type(call->func, ret));
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
Expand All @@ -4073,7 +4080,7 @@ ZEND_VM_HOT_HANDLER(60, ZEND_DO_FCALL, ANY, ANY, SPEC(RETVAL))
}
}

ZEND_VM_C_LABEL(fcall_end):
ZEND_VM_C_LABEL(fcall_end): ZEND_ATTRIBUTE_UNUSED;
if (UNEXPECTED(ZEND_CALL_INFO(call) & ZEND_CALL_RELEASE_THIS)) {
OBJ_RELEASE(Z_OBJ(call->This));
}
Expand Down Expand Up @@ -8108,14 +8115,14 @@ ZEND_VM_HANDLER(158, ZEND_CALL_TRAMPOLINE, ANY, ANY)

EG(current_execute_data) = call;

if (UNEXPECTED(fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS)
&& UNEXPECTED(!zend_verify_internal_arg_types(fbc, call))) {
zend_vm_stack_free_call_frame(call);
if (ret) {
ZVAL_UNDEF(ret);
}
ZEND_VM_C_GOTO(call_trampoline_end);
}
#if ZEND_DEBUG
/* Type checks for internal functions are usually only performed by zpp.
* In debug mode we additionally run arginfo checks to detect cases where
* arginfo and zpp went out of sync. */
zend_bool wrong_arg_types =
(fbc->common.fn_flags & ZEND_ACC_HAS_TYPE_HINTS) &&
!zend_verify_internal_arg_types(fbc, call);
#endif

if (ret == NULL) {
ZVAL_NULL(&retval);
Expand All @@ -8131,6 +8138,7 @@ ZEND_VM_HANDLER(158, ZEND_CALL_TRAMPOLINE, ANY, ANY)

#if ZEND_DEBUG
if (!EG(exception) && call->func) {
ZEND_ASSERT(!wrong_arg_types && "Arginfo / zpp type mismatch?");
ZEND_ASSERT(!(call->func->common.fn_flags & ZEND_ACC_HAS_RETURN_TYPE) ||
zend_verify_internal_return_type(call->func, ret));
ZEND_ASSERT((call->func->common.fn_flags & ZEND_ACC_RETURN_REFERENCE)
Expand All @@ -8147,7 +8155,7 @@ ZEND_VM_HANDLER(158, ZEND_CALL_TRAMPOLINE, ANY, ANY)
}
}

ZEND_VM_C_LABEL(call_trampoline_end):
ZEND_VM_C_LABEL(call_trampoline_end): ZEND_ATTRIBUTE_UNUSED;
execute_data = EG(current_execute_data);

if (!EX(func) || !ZEND_USER_CODE(EX(func)->type) || (call_info & ZEND_CALL_TOP)) {
Expand Down
Loading