From 737a7783fbbdc7bbc2826119f4eb4e9347d77f1c Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Tue, 2 Nov 2021 10:04:56 +1300 Subject: [PATCH] Add define to optionally enable escape-on-output This adjusts the smarty class such that if you have the define below escape-on-output is enabled This PR + fixing the use of isset in the template layer are the pre-requisites for turning escape on output on and off. Keeping this part in an unmerged branch is tricky as the templates need to be recompiled when you switch to a branch which does not have this function I envisage us starting to use this define in our dev environments fairly soon as it's working well locally for me on https://github.com/civicrm/civicrm-core/pull/21935 --- CRM/Core/Smarty.php | 90 +++++++++++++++++++ .../CRM/common/civicrm.settings.php.template | 14 ++- 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/CRM/Core/Smarty.php b/CRM/Core/Smarty.php index 9f805c9c8d..fa9f36f83c 100644 --- a/CRM/Core/Smarty.php +++ b/CRM/Core/Smarty.php @@ -138,6 +138,12 @@ class CRM_Core_Smarty extends Smarty { } $this->register_function('crmURL', ['CRM_Utils_System', 'crmURL']); + if (CRM_Utils_Constant::value('CIVICRM_SMARTY_DEFAULT_ESCAPE')) { + if (!isset($this->_plugins['modifier']['escape'])) { + $this->register_modifier('escape', ['CRM_Core_Smarty', 'escape']); + } + $this->default_modifiers[] = 'escape:"htmlall"'; + } $this->load_filter('pre', 'resetExtScope'); $this->assign('crmPermissions', new CRM_Core_Smarty_Permissions()); @@ -348,4 +354,88 @@ class CRM_Core_Smarty extends Smarty { return CRM_Utils_Constant::value('CIVICRM_TEMPLATE_COMPILE_CHECK', TRUE); } + /** + * Smarty escape modifier plugin. + * + * This replaces the core smarty modifier and basically does a lot of + * early-returning before calling the core function. + * + * It early returns on patterns that are common 'no-escape' patterns + * in CiviCRM - this list can be honed over time. + * + * It also logs anything that is actually escaped. Since this only kicks + * in when CIVICRM_SMARTY_DEFAULT_ESCAPE is defined it is ok to be aggressive + * about logging as we mostly care about developers using it at this stage. + * + * Note we don't actually use 'htmlall' anywhere in our tpl layer yet so + * anything coming in with this be happening because of the default modifier. + * + * Also note the right way to opt a field OUT of escaping is + * ``{$fieldName|smarty:nodefaults}`` + * This should be used for fields with known html AND for fields where + * we are doing empty or isset checks - as otherwise the value is passed for + * escaping first so you still get an enotice for 'empty' or a fatal for 'isset' + * + * Type: modifier
+ * Name: escape
+ * Purpose: Escape the string according to escapement type + * + * @link http://smarty.php.net/manual/en/language.modifier.escape.php + * escape (Smarty online manual) + * @author Monte Ohrt + * + * @param string $string + * @param string $esc_type + * @param string $char_set + * + * @return string + */ + public static function escape($string, $esc_type = 'html', $char_set = 'ISO-8859-1') { + // CiviCRM variables are often arrays - just handle them. + // The early return on booleans & numbers is mostly to prevent them being + // logged as 'changed' when they are cast to a string. + if (!is_scalar($string) || empty($string) || is_bool($string) || is_numeric($string) || $esc_type === 'none') { + return $string; + } + if ($esc_type === 'htmlall') { + // 'htmlall' is the nothing-specified default. + // Don't escape things we think quickform added. + if (strpos($string, '') === 0 + || strpos($string, 'Tournament Fees is a required field. + || strpos($string, ' + ') === 0 + // e.g from participant tab class="action-item" href=/civicrm/contact/view/participant?reset=1&action=add&cid=142&context=participant + || strpos($string, 'class="action-item" href=/civicrm/"') === 0 + ) { + // Do not escape the above common patterns. + return $string; + } + } + require_once 'Smarty/plugins/modifier.escape.php'; + $value = smarty_modifier_escape($string, $esc_type, $char_set); + if ($value !== $string) { + Civi::log()->debug('smarty escaping original {original}, escaped {escaped} type {type} charset {charset}', [ + 'original' => $string, + 'escaped' => $value, + 'type' => $esc_type, + 'charset' => $char_set, + ]); + } + return $value; + } + } diff --git a/templates/CRM/common/civicrm.settings.php.template b/templates/CRM/common/civicrm.settings.php.template index 1747c0e72d..2ee27829d9 100644 --- a/templates/CRM/common/civicrm.settings.php.template +++ b/templates/CRM/common/civicrm.settings.php.template @@ -193,7 +193,19 @@ if (!defined('CIVICRM_TEMPLATE_COMPILEDIR')) { * */ //if (!defined('CIVICRM_TEMPLATE_COMPILE_CHECK')) { -// define( 'CIVICRM_TEMPLATE_COMPILE_CHECK', FALSE); +// define('CIVICRM_TEMPLATE_COMPILE_CHECK', FALSE); +//} + +/** + * Smarty escape on output. + * + * This tells smarty to pass all variables through the escape function + * unless they are piped to smarty:nodefaults (eg. {$myScript|smarty:nodefaults} + * At this stage it should only be enabled on development sites. + * @see https://github.com/civicrm/civicrm-core/pull/21935 + */ +//if (!defined('CIVICRM_SMARTY_DEFAULT_ESCAPE')) { +// define('CIVICRM_SMARTY_DEFAULT_ESCAPE', TRUE); //} /** -- 2.25.1