From 8f96abbc6f6f39a10923f01948fcc3fe01ed0344 Mon Sep 17 00:00:00 2001 From: Vladimir Kolesnikov Date: Fri, 29 Nov 2013 17:09:37 +0200 Subject: [PATCH 1/2] Disallow to override superglobals --- ext/config.m4 | 1 + ext/config.w32 | 2 +- ext/mvc/view/engine/helpers.c | 30 ++++++++++++ ext/mvc/view/engine/helpers.h | 27 +++++++++++ ext/mvc/view/engine/php.c | 55 +++++++++++----------- ext/mvc/view/engine/volt.c | 88 +++++++++++++++-------------------- 6 files changed, 122 insertions(+), 81 deletions(-) create mode 100644 ext/mvc/view/engine/helpers.c create mode 100644 ext/mvc/view/engine/helpers.h diff --git a/ext/config.m4 b/ext/config.m4 index 80dc8c479bb..8a16a82ef14 100644 --- a/ext/config.m4 +++ b/ext/config.m4 @@ -137,6 +137,7 @@ mvc/collectioninterface.c \ mvc/view/engine/php.c \ mvc/view/engine/volt/compiler.c \ mvc/view/engine/volt.c \ +mvc/view/engine/helpers.c \ mvc/view/exception.c \ mvc/view/engineinterface.c \ mvc/view/simple.c \ diff --git a/ext/config.w32 b/ext/config.w32 index 8e973ed0701..1c51d73615a 100644 --- a/ext/config.w32 +++ b/ext/config.w32 @@ -35,7 +35,7 @@ if (PHP_PHALCON != "no") { ADD_SOURCES("ext/phalcon/mvc/user", "component.c plugin.c module.c", "phalcon") ADD_SOURCES("ext/phalcon/mvc/router", "group.c route.c annotations.c exception.c routeinterface.c", "phalcon") ADD_SOURCES("ext/phalcon/mvc/url", "exception.c", "phalcon") - ADD_SOURCES("ext/phalcon/mvc/view/engine", "php.c volt.c", "phalcon") + ADD_SOURCES("ext/phalcon/mvc/view/engine", "php.c volt.c helpers.c", "phalcon") ADD_SOURCES("ext/phalcon/mvc/view", "exception.c engineinterface.c simple.c engine.c", "phalcon") ADD_SOURCES("ext/phalcon/mvc/model/metadata", "files.c apc.c xcache.c memory.c session.c", "phalcon") ADD_SOURCES("ext/phalcon/mvc/model/metadata/strategy", "introspection.c annotations.c", "phalcon") diff --git a/ext/mvc/view/engine/helpers.c b/ext/mvc/view/engine/helpers.c new file mode 100644 index 00000000000..a129ef754b0 --- /dev/null +++ b/ext/mvc/view/engine/helpers.c @@ -0,0 +1,30 @@ +/* + +------------------------------------------------------------------------+ + | Phalcon Framework | + +------------------------------------------------------------------------+ + | Copyright (c) 2011-2013 Phalcon Team (http://www.phalconphp.com) | + +------------------------------------------------------------------------+ + | This source file is subject to the New BSD License that is bundled | + | with this package in the file docs/LICENSE.txt. | + | | + | If you did not receive a copy of the license and are unable to | + | obtain it through the world-wide-web, please send an email | + | to license@phalconphp.com so we can send you a copy immediately. | + +------------------------------------------------------------------------+ + | Authors: Andres Gutierrez | + | Eduar Carvajal | + | Vladimir Kolesnikov | + +------------------------------------------------------------------------+ +*/ + +#include "mvc/view/engine/helpers.h" +#include + +zend_bool phalcon_mvc_view_engine_php_symtable_merger(HashTable *ht, void *pData, zend_hash_key *hash_key, void *pParam) +{ +#ifdef ZTS + TSRMLS_FETCH_FROM_CTX(pParam); +#endif + + return hash_key->arKey && hash_key->nKeyLength && !zend_hash_quick_exists(CG(auto_globals), hash_key->arKey, hash_key->nKeyLength, hash_key->h); +} diff --git a/ext/mvc/view/engine/helpers.h b/ext/mvc/view/engine/helpers.h new file mode 100644 index 00000000000..ea4e82446ff --- /dev/null +++ b/ext/mvc/view/engine/helpers.h @@ -0,0 +1,27 @@ +/* + +------------------------------------------------------------------------+ + | Phalcon Framework | + +------------------------------------------------------------------------+ + | Copyright (c) 2011-2013 Phalcon Team (http://www.phalconphp.com) | + +------------------------------------------------------------------------+ + | This source file is subject to the New BSD License that is bundled | + | with this package in the file docs/LICENSE.txt. | + | | + | If you did not receive a copy of the license and are unable to | + | obtain it through the world-wide-web, please send an email | + | to license@phalconphp.com so we can send you a copy immediately. | + +------------------------------------------------------------------------+ + | Authors: Andres Gutierrez | + | Eduar Carvajal | + | Vladimir Kolesnikov | + +------------------------------------------------------------------------+ +*/ + +#ifndef PHALCON_MVC_VIEW_ENGINE_HELPERS_H +#define PHALCON_MVC_VIEW_ENGINE_HELPERS_H + +#include + +zend_bool phalcon_mvc_view_engine_php_symtable_merger(HashTable *ht, void *pData, zend_hash_key *hash_key, void *pParam); + +#endif /* PHALCON_MBC_VIEW_ENGINE_HELPERS_H */ diff --git a/ext/mvc/view/engine/php.c b/ext/mvc/view/engine/php.c index 2842f8168b7..520093c7efd 100644 --- a/ext/mvc/view/engine/php.c +++ b/ext/mvc/view/engine/php.c @@ -39,6 +39,8 @@ #include "kernel/require.h" #include "kernel/object.h" +#include "mvc/view/engine/helpers.h" + /** * Phalcon\Mvc\View\Engine\Php * @@ -67,15 +69,10 @@ PHALCON_INIT_CLASS(Phalcon_Mvc_View_Engine_Php){ */ PHP_METHOD(Phalcon_Mvc_View_Engine_Php, render){ - zval *path, *params, *must_clean = NULL, *value = NULL, *key = NULL, *contents; + zval *path, *params, *must_clean = NULL, *contents; zval *view; - HashTable *ah0; - HashPosition hp0; - zval **hd; - - PHALCON_MM_GROW(); - phalcon_fetch_params(1, 2, 1, &path, ¶ms, &must_clean); + phalcon_fetch_params(0, 2, 1, &path, ¶ms, &must_clean); if (!must_clean) { must_clean = PHALCON_GLOBAL(z_false); @@ -89,38 +86,38 @@ PHP_METHOD(Phalcon_Mvc_View_Engine_Php, render){ * Create the variables in local symbol table */ if (Z_TYPE_P(params) == IS_ARRAY) { - - phalcon_is_iterable(params, &ah0, &hp0, 0, 0); - - while (zend_hash_get_current_data_ex(ah0, (void**) &hd, &hp0) == SUCCESS) { - - PHALCON_GET_HKEY(key, ah0, hp0); - PHALCON_GET_HVALUE(value); - - if (phalcon_set_symbol(key, value TSRMLS_CC) == FAILURE){ - return; - } - - zend_hash_move_forward_ex(ah0, &hp0); + if (!EG(active_symbol_table)) { + zend_rebuild_symbol_table(TSRMLS_C); } - + + zend_hash_merge_ex( + EG(active_symbol_table), + Z_ARRVAL_P(params), + (copy_ctor_func_t)zval_add_ref, + sizeof(zval*), + phalcon_mvc_view_engine_php_symtable_merger +#ifdef ZTS + TSRMLS_CC +#else + NULL +#endif + ); } /** * Require the file */ if (phalcon_require(path TSRMLS_CC) == FAILURE) { - RETURN_MM(); + RETURN_FALSE; } + if (PHALCON_IS_TRUE(must_clean)) { - PHALCON_INIT_VAR(contents); + PHALCON_ALLOC_GHOST_ZVAL(contents); phalcon_ob_get_contents(contents TSRMLS_CC); - PHALCON_OBS_VAR(view); - phalcon_read_property_this(&view, this_ptr, SL("_view"), PH_NOISY_CC); - phalcon_call_method_p1_noret(view, "setcontent", contents); + view = phalcon_fetch_nproperty_this(this_ptr, SL("_view"), PH_NOISY_CC); + phalcon_call_method_params(NULL, NULL, view, SL("setcontent"), zend_inline_hash_func(SS("setcontent")) TSRMLS_CC, 1, contents); } - - PHALCON_MM_RESTORE(); -} + RETURN_TRUE; +} diff --git a/ext/mvc/view/engine/volt.c b/ext/mvc/view/engine/volt.c index f336dc90776..bb44a59241a 100644 --- a/ext/mvc/view/engine/volt.c +++ b/ext/mvc/view/engine/volt.c @@ -42,6 +42,8 @@ #include "kernel/string.h" #include "kernel/array.h" +#include "mvc/view/engine/helpers.h" + /** * Phalcon\Mvc\View\Engine\Volt * @@ -152,11 +154,8 @@ PHP_METHOD(Phalcon_Mvc_View_Engine_Volt, getCompiler){ PHP_METHOD(Phalcon_Mvc_View_Engine_Volt, render){ zval *template_path, *params, *must_clean = NULL, *compiler; - zval *compiled_template_path, *value = NULL, *key = NULL, *contents; + zval *compiled_template_path, *contents; zval *view; - HashTable *ah0; - HashPosition hp0; - zval **hd; PHALCON_MM_GROW(); @@ -181,38 +180,40 @@ PHP_METHOD(Phalcon_Mvc_View_Engine_Volt, render){ phalcon_call_method(compiled_template_path, compiler, "getcompiledtemplatepath"); /** - * Export the variables the current symbol table + * Export the variables into the current symbol table */ if (Z_TYPE_P(params) == IS_ARRAY) { - - phalcon_is_iterable(params, &ah0, &hp0, 0, 0); - - while (zend_hash_get_current_data_ex(ah0, (void**) &hd, &hp0) == SUCCESS) { - - PHALCON_GET_HKEY(key, ah0, hp0); - PHALCON_GET_HVALUE(value); - - if (phalcon_set_symbol(key, value TSRMLS_CC) == FAILURE){ - return; - } - - zend_hash_move_forward_ex(ah0, &hp0); + if (!EG(active_symbol_table)) { + zend_rebuild_symbol_table(TSRMLS_C); } - + + zend_hash_merge_ex( + EG(active_symbol_table), + Z_ARRVAL_P(params), + (copy_ctor_func_t)zval_add_ref, + sizeof(zval*), + phalcon_mvc_view_engine_php_symtable_merger +#ifdef ZTS + TSRMLS_CC +#else + NULL +#endif + ); } if (phalcon_require(compiled_template_path TSRMLS_CC) == FAILURE) { + RETVAL_FALSE; RETURN_MM(); } if (PHALCON_IS_TRUE(must_clean)) { PHALCON_INIT_VAR(contents); phalcon_ob_get_contents(contents TSRMLS_CC); - PHALCON_OBS_VAR(view); - phalcon_read_property_this(&view, this_ptr, SL("_view"), PH_NOISY_CC); + view = phalcon_fetch_nproperty_this(this_ptr, SL("_view"), PH_NOISY_CC); phalcon_call_method_p1_noret(view, "setcontent", contents); } + RETVAL_TRUE; PHALCON_MM_RESTORE(); } @@ -224,32 +225,19 @@ PHP_METHOD(Phalcon_Mvc_View_Engine_Volt, render){ */ PHP_METHOD(Phalcon_Mvc_View_Engine_Volt, length){ - zval *item, *length = NULL; - - PHALCON_MM_GROW(); + zval *item; - phalcon_fetch_params(1, 1, 0, &item); + phalcon_fetch_params(0, 1, 0, &item); - PHALCON_INIT_VAR(length); - ZVAL_LONG(length, 0); - if (Z_TYPE_P(item) == IS_OBJECT) { - phalcon_fast_count(length, item TSRMLS_CC); + if (Z_TYPE_P(item) == IS_OBJECT || Z_TYPE_P(item) == IS_ARRAY) { + phalcon_fast_count(return_value, item TSRMLS_CC); + } else if (phalcon_function_exists_ex(SS("mb_strlen") TSRMLS_CC) == SUCCESS) { + PHALCON_MM_GROW(); + phalcon_call_func_p1(return_value, "mb_strlen", item); + PHALCON_MM_RESTORE(); } else { - if (Z_TYPE_P(item) == IS_ARRAY) { - PHALCON_INIT_NVAR(length); - phalcon_fast_count(length, item TSRMLS_CC); - } else { - if (phalcon_function_exists_ex(SS("mb_strlen") TSRMLS_CC) == SUCCESS) { - PHALCON_INIT_NVAR(length); - phalcon_call_func_p1(length, "mb_strlen", item); - } else { - PHALCON_INIT_NVAR(length); - phalcon_fast_strlen(length, item); - } - } + phalcon_fast_strlen(return_value, item); } - - RETURN_CCTOR(length); } /** @@ -263,25 +251,23 @@ PHP_METHOD(Phalcon_Mvc_View_Engine_Volt, isIncluded){ zval *needle, *haystack; - PHALCON_MM_GROW(); - - phalcon_fetch_params(1, 2, 0, &needle, &haystack); + phalcon_fetch_params(0, 2, 0, &needle, &haystack); if (Z_TYPE_P(haystack) == IS_ARRAY) { - RETVAL_BOOL(phalcon_fast_in_array(needle, haystack TSRMLS_CC)); - RETURN_MM(); + RETURN_BOOL(phalcon_fast_in_array(needle, haystack TSRMLS_CC)); } + if (Z_TYPE_P(haystack) == IS_STRING) { if (phalcon_function_exists_ex(SS("mb_strpos") TSRMLS_CC) == SUCCESS) { + PHALCON_MM_GROW(); phalcon_call_func_p2(return_value, "mb_strpos", haystack, needle); RETURN_MM(); } + phalcon_fast_strpos(return_value, haystack, needle); - RETURN_MM(); } - PHALCON_THROW_EXCEPTION_STR(phalcon_mvc_view_exception_ce, "Invalid haystack"); - return; + PHALCON_THROW_EXCEPTION_STRW(phalcon_mvc_view_exception_ce, "Invalid haystack"); } /** From bbff6d5c6be11c550b6e67f05c6680cd1145db81 Mon Sep 17 00:00:00 2001 From: Vladimir Kolesnikov Date: Sat, 30 Nov 2013 04:18:13 +0200 Subject: [PATCH 2/2] Test case for #1617 --- ext/tests/issue-1617.phpt | 33 ++++++++++++++++++++++++++++++ ext/tests/views/superglobals.phtml | 5 +++++ 2 files changed, 38 insertions(+) create mode 100644 ext/tests/issue-1617.phpt create mode 100644 ext/tests/views/superglobals.phtml diff --git a/ext/tests/issue-1617.phpt b/ext/tests/issue-1617.phpt new file mode 100644 index 00000000000..852b68a5c6b --- /dev/null +++ b/ext/tests/issue-1617.phpt @@ -0,0 +1,33 @@ +--TEST-- +Do not allow to override superglobals in views - https://github.com/phalcon/cphalcon/pull/1617 +--SKIPIF-- + +--FILE-- +setVar($v, strtolower($v)); +} + +var_dump($view->getParamsToView()); +var_dump($engine->render(__DIR__ . '/views/superglobals.phtml', $view->getParamsToView(), false)); +?> +--EXPECT-- +array(4) { + ["_GET"]=> + string(4) "_get" + ["_POST"]=> + string(5) "_post" + ["_COOKIE"]=> + string(7) "_cookie" + ["GLOBALS"]=> + string(7) "globals" +} +array +array +array +array +bool(true) diff --git a/ext/tests/views/superglobals.phtml b/ext/tests/views/superglobals.phtml new file mode 100644 index 00000000000..cdf05a2c48c --- /dev/null +++ b/ext/tests/views/superglobals.phtml @@ -0,0 +1,5 @@ +