Re: [ZEND-ENGINE-CVS] cvs: ZendEngine2(PHP_5_2) / zend.h zend_API.c zend_builtin_functions.c zend_compile.c zend_execute_API.c zend_object_handlers.c /tests bug43128.phpt php-src NEWS
[email protected] (Marcus Boerger)
| Newsgroups | php.zend-engine.cvs |
|---|---|
| Message-ID | <[email protected]> |
Hello Dmitry,
Friday, February 15, 2008, 4:34:26 PM, you wrote:
> I didn't like to backport this patch into PHP_5_2, because it cannot be
> done in the same clear way as in PHP_5_3 and HEAD without BC break.
> I did it just because Ilia asked me to fix this security issue.
> a) performance penalty, but it may be good solution.
> b) long class-name is just one possible way to reach stack overflow
> I didn't understand what kind of useless variables you mean. use_heap?
exactly, just replace those calls with malloc/emalloc. That is a)
undersatndable, b) maintainable and c) probably faster.
marcus
> Thanks. Dmitry.
> Marcus Boerger wrote:
>> Hello Dmitry,
>>
>> it appears to me we are making it overly complex. I see to much easier
>> solutions:
>>
>> a) replace alloca and friends with malloc
>> b) disallox function/class names longer than that limit
>>
>> oh and both versions would get rid of tons of newly introduced useless
>> variables
>>
>> marcus
>>
>> Friday, February 15, 2008, 8:44:46 AM, you wrote:
>>
>>> http://cvs.php.net/viewvc.cgi/php-src/NEWS?r1=1.2027.2.547.2.1080&r2=1.2027.2.547.2.1081&diff_format=u
>>> Index: php-src/NEWS
>>> diff -u php-src/NEWS:1.2027.2.547.2.1080 php-src/NEWS:1.2027.2.547.2.1081
>>> --- php-src/NEWS:1.2027.2.547.2.1080 Fri Feb 15 06:50:39 2008
>>> +++ php-src/NEWS Fri Feb 15 07:44:45 2008
>>> @@ -95,6 +95,7 @@
>>> truncation). (Ilia)
>>> - Fixed bug #43175 (__destruct() throwing an exception with __call() causes
>>> segfault). (Dmitry)
>>> +- Fixed bug #43128 (Very long class name causes segfault). (Dmitry)
>>> - Fixed bug #43105 (PHP seems to fail to close open files). (Hannes)
>>> - Fixed bug #42978 (mismatch between number of bound params and values causes
>>> a crash in pdo_pgsql). (Ilia)
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend.h?r1=1.293.2.11.2.10&r2=1.293.2.11.2.11&diff_format=u
>>> Index: ZendEngine2/zend.h
>>> diff -u ZendEngine2/zend.h:1.293.2.11.2.10 ZendEngine2/zend.h:1.293.2.11.2.11
>>> --- ZendEngine2/zend.h:1.293.2.11.2.10 Mon Dec 31 07:20:02 2007
>>> +++ ZendEngine2/zend.h Fri Feb 15 07:44:45 2008
>>> @@ -17,7 +17,7 @@
>>> +----------------------------------------------------------------------+
>>> */
>>>
>>> -/* $Id: zend.h,v 1.293.2.11.2.10 2007/12/31 07:20:02 sebastian Exp $ */
>>> +/* $Id: zend.h,v 1.293.2.11.2.11 2008/02/15 07:44:45 dmitry Exp $ */
>>>
>>> #ifndef ZEND_H
>>> #define ZEND_H
>>> @@ -181,9 +181,21 @@
>>> #if (HAVE_ALLOCA || (defined (__GNUC__) && __GNUC__ >= 2)) && !(defined(ZTS) &&
>>> defined(ZEND_WIN32)) && !(defined(ZTS) && defined(NETWARE)) && !(defined(ZTS) &&
>>> defined(HPUX)) && !defined(DARWIN)
>>> # define do_alloca(p) alloca(p)
>>> # define free_alloca(p)
>>> +# define ZEND_ALLOCA_MAX_SIZE (32 * 1024)
>>> +# define ALLOCA_FLAG(name) \
>>> + zend_bool name;
>>> +# define do_alloca_with_limit_ex(size, limit, use_heap) \
>>> + ((use_heap = ((size) > (limit))) ? emalloc(size) : alloca(size))
>>> +# define do_alloca_with_limit(size, use_heap) \
>>> + do_alloca_with_limit_ex(size, ZEND_ALLOCA_MAX_SIZE, use_heap)
>>> +# define free_alloca_with_limit(p, use_heap) \
>>> + do { if (use_heap) efree(p); } while (0)
>>> #else
>>> # define do_alloca(p) emalloc(p)
>>> # define free_alloca(p) efree(p)
>>> +# define ALLOCA_FLAG(name)
>>> +# define do_alloca_with_limit(p, use_heap) emalloc(p)
>>> +# define free_alloca_with_limit(p, use_heap) efree(p)
>>> #endif
>>>
>>> #if ZEND_DEBUG
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_API.c?r1=1.296.2.27.2.35&r2=1.296.2.27.2.36&diff_format=u
>>> Index: ZendEngine2/zend_API.c
>>> diff -u ZendEngine2/zend_API.c:1.296.2.27.2.35
>>> ZendEngine2/zend_API.c:1.296.2.27.2.36
>>> --- ZendEngine2/zend_API.c:1.296.2.27.2.35 Mon Dec 31 07:20:02 2007
>>> +++ ZendEngine2/zend_API.c Fri Feb 15 07:44:45 2008
>>> @@ -18,7 +18,7 @@
>>> +----------------------------------------------------------------------+
>>> */
>>>
>>> -/* $Id: zend_API.c,v 1.296.2.27.2.35 2007/12/31 07:20:02 sebastian Exp $ */
>>> +/* $Id: zend_API.c,v 1.296.2.27.2.36 2008/02/15 07:44:45 dmitry Exp $ */
>>>
>>> #include "zend.h"
>>> #include "zend_execute.h"
>>> @@ -1723,11 +1723,10 @@
>>> }
>>> }
>>> fname_len = strlen(ptr->fname);
>>> - lowercase_name = do_alloca(fname_len+1);
>>> - zend_str_tolower_copy(lowercase_name, ptr->fname, fname_len);
>>> + lowercase_name = zend_str_tolower_dup(ptr->fname, fname_len);
>>> if (zend_hash_add(target_function_table, lowercase_name, fname_len+1,
>>> &function, sizeof(zend_function), (void**)®_function) == FAILURE) {
>>> unload=1;
>>> - free_alloca(lowercase_name);
>>> + efree(lowercase_name);
>>> break;
>>> }
>>> if (scope) {
>>> @@ -1767,7 +1766,7 @@
>>> }
>>> ptr++;
>>> count++;
>>> - free_alloca(lowercase_name);
>>> + efree(lowercase_name);
>>> }
>>> if (unload) { /* before unloading, display all remaining bad function in the module
>>> */
>>> if (scope) {
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_builtin_functions.c?r1=1.277.2.12.2.26&r2=1.277.2.12.2.27&diff_format=u
>>> Index: ZendEngine2/zend_builtin_functions.c
>>> diff -u ZendEngine2/zend_builtin_functions.c:1.277.2.12.2.26
>>> ZendEngine2/zend_builtin_functions.c:1.277.2.12.2.27
>>> --- ZendEngine2/zend_builtin_functions.c:1.277.2.12.2.26 Mon Dec 31 07:20:02 2007
>>> +++ ZendEngine2/zend_builtin_functions.c Fri Feb 15 07:44:45 2008
>>> @@ -17,7 +17,7 @@
>>> +----------------------------------------------------------------------+
>>> */
>>>
>>> -/* $Id: zend_builtin_functions.c,v 1.277.2.12.2.26 2007/12/31 07:20:02 sebastian Exp $ */
>>> +/* $Id: zend_builtin_functions.c,v 1.277.2.12.2.27 2008/02/15 07:44:45 dmitry Exp $ */
>>>
>>> #include "zend.h"
>>> #include "zend_API.h"
>>> @@ -1013,19 +1013,20 @@
>>> char *class_name, *lc_name;
>>> zend_class_entry **ce;
>>> int class_name_len;
>>> - zend_bool autoload = 1;
>>> int found;
>>> + zend_bool autoload = 1;
>>> + ALLOCA_FLAG(use_heap)
>>>
>>> if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|b", &class_name,
>>> &class_name_len, &autoload) == FAILURE) {
>>> return;
>>> }
>>>
>>> if (!autoload) {
>>> - lc_name = do_alloca(class_name_len + 1);
>>> + lc_name = do_alloca_with_limit(class_name_len + 1, use_heap);
>>> zend_str_tolower_copy(lc_name, class_name, class_name_len);
>>>
>>> found = zend_hash_find(EG(class_table), lc_name, class_name_len+1, (void **)
>>> &ce);
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> RETURN_BOOL(found == SUCCESS && !((*ce)->ce_flags & ZEND_ACC_INTERFACE));
>>> }
>>>
>>> @@ -1044,19 +1045,20 @@
>>> char *iface_name, *lc_name;
>>> zend_class_entry **ce;
>>> int iface_name_len;
>>> - zend_bool autoload = 1;
>>> int found;
>>> + zend_bool autoload = 1;
>>> + ALLOCA_FLAG(use_heap)
>>>
>>> if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|b", &iface_name,
>>> &iface_name_len, &autoload) == FAILURE) {
>>> return;
>>> }
>>>
>>> if (!autoload) {
>>> - lc_name = do_alloca(iface_name_len + 1);
>>> + lc_name = do_alloca_with_limit(iface_name_len + 1, use_heap);
>>> zend_str_tolower_copy(lc_name, iface_name, iface_name_len);
>>>
>>> found = zend_hash_find(EG(class_table), lc_name, iface_name_len+1, (void **)
>>> &ce);
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> RETURN_BOOL(found == SUCCESS && (*ce)->ce_flags & ZEND_ACC_INTERFACE);
>>> }
>>>
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_compile.c?r1=1.647.2.27.2.46&r2=1.647.2.27.2.47&diff_format=u
>>> Index: ZendEngine2/zend_compile.c
>>> diff -u ZendEngine2/zend_compile.c:1.647.2.27.2.46
>>> ZendEngine2/zend_compile.c:1.647.2.27.2.47
>>> --- ZendEngine2/zend_compile.c:1.647.2.27.2.46 Tue Jan 29 00:08:08 2008
>>> +++ ZendEngine2/zend_compile.c Fri Feb 15 07:44:45 2008
>>> @@ -17,7 +17,7 @@
>>> +----------------------------------------------------------------------+
>>> */
>>>
>>> -/* $Id: zend_compile.c,v 1.647.2.27.2.46 2008/01/29 00:08:08 tony2001 Exp $ */
>>> +/* $Id: zend_compile.c,v 1.647.2.27.2.47 2008/02/15 07:44:45 dmitry Exp $ */
>>>
>>> #include <zend_language_parser.h>
>>> #include "zend.h"
>>> @@ -1100,6 +1100,7 @@
>>> zend_uint fn_flags;
>>> char *lcname;
>>> zend_bool orig_interactive;
>>> + ALLOCA_FLAG(use_heap)
>>>
>>> if (is_method) {
>>> if (CG(active_class_entry)->ce_flags & ZEND_ACC_INTERFACE) {
>>> @@ -1160,7 +1161,7 @@
>>> }
>>>
>>> if (!(CG(active_class_entry)->ce_flags & ZEND_ACC_INTERFACE)) {
>>> - short_class_name =
>>> do_alloca(short_class_name_length + 1);
>>> + short_class_name =
>>> do_alloca_with_limit(short_class_name_length + 1,
>>> use_heap);
>>> zend_str_tolower_copy(short_class_name,
>>> CG(active_class_entry)->name, short_class_name_length);
>>> /* Improve after RC: cache the lowercase class name */
>>>
>>> @@ -1194,7 +1195,7 @@
>>> } else if (!(fn_flags & ZEND_ACC_STATIC)) {
>>> CG(active_op_array)->fn_flags |= ZEND_ACC_ALLOW_STATIC;
>>> }
>>> - free_alloca(short_class_name);
>>> + free_alloca_with_limit(short_class_name, use_heap);
>>> }
>>>
>>> efree(lcname);
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_execute_API.c?r1=1.331.2.20.2.25&r2=1.331.2.20.2.26&diff_format=u
>>> Index: ZendEngine2/zend_execute_API.c
>>> diff -u ZendEngine2/zend_execute_API.c:1.331.2.20.2.25
>>> ZendEngine2/zend_execute_API.c:1.331.2.20.2.26
>>> --- ZendEngine2/zend_execute_API.c:1.331.2.20.2.25 Mon Dec 31 07:20:02 2007
>>> +++ ZendEngine2/zend_execute_API.c Fri Feb 15 07:44:45 2008
>>> @@ -17,7 +17,7 @@
>>> +----------------------------------------------------------------------+
>>> */
>>>
>>> -/* $Id: zend_execute_API.c,v 1.331.2.20.2.25 2007/12/31 07:20:02 sebastian Exp $ */
>>> +/* $Id: zend_execute_API.c,v 1.331.2.20.2.26 2008/02/15 07:44:45 dmitry Exp $ */
>>>
>>> #include <stdio.h>
>>> #include <signal.h>
>>> @@ -1034,19 +1034,20 @@
>>> int retval;
>>> char *lc_name;
>>> zval *exception;
>>> - char dummy = 1;
>>> zend_fcall_info fcall_info;
>>> zend_fcall_info_cache fcall_cache;
>>> + char dummy = 1;
>>> + ALLOCA_FLAG(use_heap)
>>>
>>> if (name == NULL || !name_length) {
>>> return FAILURE;
>>> }
>>>
>>> - lc_name = do_alloca(name_length + 1);
>>> + lc_name = do_alloca_with_limit(name_length + 1, use_heap);
>>> zend_str_tolower_copy(lc_name, name, name_length);
>>>
>>> if (zend_hash_find(EG(class_table), lc_name, name_length+1, (void **) ce) ==
>>> SUCCESS) {
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> return SUCCESS;
>>> }
>>>
>>> @@ -1054,7 +1055,7 @@
>>> * (doesn't impact fuctionality of __autoload()
>>> */
>>> if (!use_autoload || zend_is_compiling(TSRMLS_C)) {
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> return FAILURE;
>>> }
>>>
>>> @@ -1064,7 +1065,7 @@
>>> }
>>>
>>> if (zend_hash_add(EG(in_autoload), lc_name, name_length+1, (void**)&dummy,
>>> sizeof(char), NULL) == FAILURE) {
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> return FAILURE;
>>> }
>>>
>>> @@ -1102,12 +1103,12 @@
>>>
>>> if (retval == FAILURE) {
>>> EG(exception) = exception;
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> return FAILURE;
>>> }
>>>
>>> if (EG(exception) && exception) {
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> zend_error(E_ERROR, "Function %s(%s) threw an exception of type '%s'",
>>> ZEND_AUTOLOAD_FUNC_NAME, name, Z_OBJCE_P(EG(exception))->name);
>>> return FAILURE;
>>> }
>>> @@ -1119,7 +1120,7 @@
>>> }
>>>
>>> retval = zend_hash_find(EG(class_table), lc_name, name_length + 1, (void **) ce);
>>> - free_alloca(lc_name);
>>> + free_alloca_with_limit(lc_name, use_heap);
>>> return retval;
>>> }
>>>
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/zend_object_handlers.c?r1=1.135.2.6.2.26&r2=1.135.2.6.2.27&diff_format=u
>>> Index: ZendEngine2/zend_object_handlers.c
>>> diff -u ZendEngine2/zend_object_handlers.c:1.135.2.6.2.26
>>> ZendEngine2/zend_object_handlers.c:1.135.2.6.2.27
>>> --- ZendEngine2/zend_object_handlers.c:1.135.2.6.2.26 Mon Dec 31 07:20:03 2007
>>> +++ ZendEngine2/zend_object_handlers.c Fri Feb 15 07:44:46 2008
>>> @@ -17,7 +17,7 @@
>>> +----------------------------------------------------------------------+
>>> */
>>>
>>> -/* $Id: zend_object_handlers.c,v 1.135.2.6.2.26 2007/12/31 07:20:03 sebastian Exp $ */
>>> +/* $Id: zend_object_handlers.c,v 1.135.2.6.2.27 2008/02/15 07:44:46 dmitry Exp $ */
>>>
>>> #include "zend.h"
>>> #include "zend_globals.h"
>>> @@ -758,14 +758,15 @@
>>> zend_function *fbc;
>>> char *lc_method_name;
>>> zval *object = *object_ptr;
>>> + ALLOCA_FLAG(use_heap)
>>>
>>> - lc_method_name = do_alloca(method_len+1);
>>> + lc_method_name = do_alloca_with_limit(method_len+1, use_heap);
>>> /* Create a zend_copy_str_tolower(dest, src, src_length); */
>>> zend_str_tolower_copy(lc_method_name, method_name, method_len);
>>>
>>> zobj = Z_OBJ_P(object);
>>> if (zend_hash_find(&zobj->ce->function_table, lc_method_name, method_len+1, (void
>>> **)&fbc) == FAILURE) {
>>> - free_alloca(lc_method_name);
>>> + free_alloca_with_limit(lc_method_name, use_heap);
>>> if (zobj->ce->__call) {
>>> zend_internal_function *call_user_call =
>>> emalloc(sizeof(zend_internal_function));
>>> call_user_call->type = ZEND_INTERNAL_FUNCTION;
>>> @@ -820,7 +821,7 @@
>>> }
>>> }
>>>
>>> - free_alloca(lc_method_name);
>>> + free_alloca_with_limit(lc_method_name, use_heap);
>>> return fbc;
>>> }
>>>
>>
>>> http://cvs.php.net/viewvc.cgi/ZendEngine2/tests/bug43128.phpt?view=markup&rev=1.1
>>> Index: ZendEngine2/tests/bug43128.phpt
>>> +++ ZendEngine2/tests/bug43128.phpt
>>
>>
>>
>>
>> Best regards,
>> Marcus
>>
Best regards,
Marcus