Skip to content
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.

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

NewInstanceOperator fails to check returned class before initiating object. #1885

Closed
davidofferman opened this issue Jul 12, 2019 · 0 comments

Comments

@davidofferman
Copy link
Contributor

commented Jul 12, 2019

Hello,
There is a problem when using the new instance operator. I believe I finally tracked down the issue that required the fix phalcon/cphalcon#14238 in cphalcon. I'm not familiar with the source code for the php interpreter or zephir, so please bear with me.

Currently, when using the new {classname}() operator in zep files, the following C code is generated.

zephir_fetch_safe_class(&_0$$3, &definition);
_1$$3 = zephir_fetch_class_str_ex(Z_STRVAL_P(&_0$$3), Z_STRLEN_P(&_0$$3), ZEND_FETCH_CLASS_AUTO);
object_init_ex(&handler, _1$$3);
if (zephir_has_constructor(&handler TSRMLS_CC)) {
    ZEPHIR_CALL_METHOD(NULL, &handler, "__construct", NULL, 0);
    zephir_check_call_status();
}

The problem is that the function zephir_fetch_class_str_ex will return NULL if the autoloaded class as a syntax error. This goes unchecked when passing it into object_init_ex which expects the handler to be valid since there isn't any guard in place.

That's why using zephir_create_instance() instead of new {classname}() doesn't cause a segfault.

Hopefully someone with more knowledge of Zephir can implement this fix. If not, I should be able to work on it in a few weeks. I'm working on a patch for this right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.