Skip to content

Commit

Permalink
COMCL-561: Apply CIVI-SA-2024-03
Browse files Browse the repository at this point in the history
Core commit: civicrm@e678b10

Included in versions 5.74.4 and 5.69.6
  • Loading branch information
Muhammad Shahrukh authored and shahrukh-compuco committed Jun 25, 2024
1 parent 34e9c1b commit a88d8bf
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 7 deletions.
207 changes: 207 additions & 0 deletions CRM/Core/Smarty/UserContentPolicy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
<?php

/**
* Define the security-constraints to apply to user-supplied Smarty content.
*
* At time of writing, we have a complication -- parallel support for Smarty 2/3/4/5. Each
* version has slightly different functionality.
*
* To bridge the gap, we define a general policy -- and then map it into each Smarty implementation.
*/
class CRM_Core_Smarty_UserContentPolicy extends \Civi\Core\Service\AutoService {

/**
* This is an array of trusted PHP functions.
* If empty all functions are allowed.
* To disable all PHP functions set $php_functions = null.
*
* @var array
*/
public $php_functions = [
'array',
'list',
'isset',
'empty',
'count',
'sizeof',
'in_array',
'is_array',
'true',
'false',
'null',
];

/**
* This is an array of trusted PHP modifiers.
* If empty all modifiers are allowed.
* To disable all modifier set $php_modifiers = null.
*
* @var array
*/
public $php_modifiers = [
'escape',
'count',
'sizeof',
'nl2br',
];

/**
* This is an array of disabled tags.
* If empty no restriction by disabled_tags.
*
* @var array
*/
public $disabled_tags = ['crmAPI'];

public $allow_constants = FALSE;

public $allow_super_globals = FALSE;

private $old_settings = NULL;

/**
* @service civi.smarty.userContent
*/
public static function create(): CRM_Core_Smarty_UserContentPolicy {
$instance = new CRM_Core_Smarty_UserContentPolicy();

$event = \Civi\Core\Event\GenericHookEvent::create(['policy' => $instance]);
Civi::dispatcher()->dispatch('hook_civicrm_userContentPolicy', $event);

return $instance;
}

public function enable(): void {
$smarty = CRM_Core_Smarty::singleton();
switch ($smarty->getVersion()) {
case 2:
$this->old_settings = $smarty->security_settings;
$smarty->security_settings = $this->createSmartyPolicy2($smarty);
$smarty->security = TRUE;
return;

case 3:
case 4:
$smarty->enableSecurity($this->createSmartyPolicy34());
return;

case 5:
$smarty->enableSecurity($this->createSmartyPolicy5());
return;
}
}

public function disable(): void {
$smarty = CRM_Core_Smarty::singleton();
switch ($smarty->getVersion()) {
case 2:
$smarty->security_settings = $this->old_settings;
$smarty->security = FALSE;
return;

case 3:
case 4:
$smarty->disableSecurity();
return;

case 5:
$smarty->disableSecurity();
return;
}
}

protected function createSmartyPolicy2($smarty): array {
$result = $smarty->security_settings;
$result['IF_FUNCS'] = $this->php_functions;
$result['MODIFIER_FUNCS'] = $this->php_modifiers;
$result['ALLOW_CONSTANTS'] = $this->allow_constants;
$result['ALLOW_SUPER_GLOBALS'] = $this->allow_super_globals;
return $result;
}

protected function createSmartyPolicy34(): string {
$obj = new class(NULL) extends Smarty_Security {

public function __construct($smarty) {
parent::__construct($smarty);

/** @var \CRM_Core_Smarty_UserContentPolicy $policy */
$policy = Civi::service('civi.smarty.userContent');

$this->php_functions = $policy->php_functions;
$this->php_modifiers = $policy->php_modifiers;
$this->disabled_tags = $policy->disabled_tags;

$this->static_classes = NULL;
$this->allow_constants = $policy->allow_constants;
$this->allow_super_globals = $policy->allow_super_globals;
}

};
return get_class($obj);
}

protected function createSmartyPolicy5(): string {

$obj = new class(NULL) extends \Smarty\Security {

public function __construct($smarty) {
if ($smarty !== NULL) {
parent::__construct($smarty);
}

/** @var \CRM_Core_Smarty_UserContentPolicy $policy */
$policy = Civi::service('civi.smarty.userContent');

// This feels counterintuitive. Eileen thinks it may be a miscommunication.
// Functionally, consider that (a) security is enabled/disabled/enabled/disabled
// but (b) the registered plugins don't actually change.

// foreach ($policy->php_functions as $phpFunction) {
// $smarty->registerPlugin('modifier', $phpFunction, $phpFunction);
// }
// foreach ($policy->php_modifiers as $modifier) {
// $smarty->registerPlugin('modifier', $modifier, $modifier);
// }

$this->static_classes = NULL;
$this->allow_constants = $policy->allow_constants;
$this->allow_super_globals = $policy->allow_super_globals;
}

};
return get_class($obj);
}

/**
* Smarty 3+4 have option to disable tags in secure mode, but Smarty 2 doesn't.
* So for any potentially-sensitive tags, we support an alternate mechanism to check access.
*
* @param string $tag
* @return void
* @throws \Exception
*/
public static function assertTagAllowed(string $tag): void {
if (!\Civi\Core\Container::isContainerBooted()) {
// We don't run user content before the system has booted.
// So the details here are kind of academic.
// Main thing: don't force a premature bootstrap.
$disabledTags = ['crmAPI'];
}
else {
$smarty = CRM_Core_Smarty::singleton();
$hasSecurity = ($smarty->getVersion() > 2) ? (bool) $smarty->security_policy : $smarty->security;
if (!$hasSecurity) {
return;
}

$policy = Civi::service('civi.smarty.userContent');
$disabledTags = $policy->disabled_tags;
}

if (in_array($tag, $disabledTags)) {
throw new \Exception("Tag '{$tag}' is not allowed in secure mode.");
}
}

}
2 changes: 2 additions & 0 deletions CRM/Core/Smarty/plugins/function.crmAPI.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
* @return string|void
*/
function smarty_function_crmAPI($params, &$smarty) {
CRM_Core_Smarty_UserContentPolicy::assertTagAllowed('crmAPI');

if (!array_key_exists('entity', $params)) {
$smarty->trigger_error("assign: missing 'entity' parameter");
return "crmAPI: missing 'entity' parameter";
Expand Down
54 changes: 48 additions & 6 deletions CRM/Utils/String.php
Original file line number Diff line number Diff line change
Expand Up @@ -1015,23 +1015,65 @@ public static function stringContainsTokens(string $string) {
* many times it is run. This compares to it otherwise creating one file for every parsed string.
*
* @param string $templateString
* @param array $templateVars
*
* @return string
*/
public static function parseOneOffStringThroughSmarty($templateString) {
public static function parseOneOffStringThroughSmarty($templateString, $templateVars = []) {
if (!CRM_Utils_String::stringContainsTokens($templateString)) {
// Skip expensive smarty processing.
return $templateString;
}
$smarty = CRM_Core_Smarty::singleton();
$cachingValue = $smarty->caching;
$smarty->caching = 0;
$useSecurityPolicy = ($smarty->getVersion() > 2) ? !$smarty->security_policy : !$smarty->security;
// For Smarty v2, policy is applied at lower level.
if ($useSecurityPolicy) {
// $smarty->enableSecurity('CRM_Core_Smarty_Security');
Civi::service('civi.smarty.userContent')->enable();
}
$smarty->assign('smartySingleUseString', $templateString);
// Do not escape the smartySingleUseString as that is our smarty template
// and is likely to contain html.
$templateString = (string) $smarty->fetch('string:{eval var=$smartySingleUseString|smarty:nodefaults}');
$smarty->caching = $cachingValue;
$smarty->assign('smartySingleUseString', NULL);
try {
// Do not escape the smartySingleUseString as that is our smarty template
// and is likely to contain html.
// The file name generated by
// 'string:{eval var=$smartySingleUseString|smarty:nodefaults}'
// is invalid in Windows, causing failure.
// Adding this is preparatory to smarty 3. The original PR failed some
// tests so we check for the function.
if (!function_exists('smarty_function_eval') && (!defined('SMARTY_DIR') || !file_exists(SMARTY_DIR . '/plugins/function.eval.php'))) {
if (!empty($templateVars)) {
$templateString = (string) $smarty->fetchWith('eval:' . $templateString, $templateVars);
}
else {
$templateString = (string) $smarty->fetch('eval:' . $templateString);
}
}
else {
if (!empty($templateVars)) {
$templateString = (string) $smarty->fetchWith('string:{eval var=$smartySingleUseString|smarty:nodefaults}', $templateVars);
}
else {
$templateString = (string) $smarty->fetch('string:{eval var=$smartySingleUseString|smarty:nodefaults}');
}
}
}
catch (Exception $e) {
\Civi::log('smarty')->info('parsing smarty template {template}', [
'template' => $templateString,
]);
throw new \CRM_Core_Exception('Message was not parsed due to invalid smarty syntax : ' . $e->getMessage() . ((CIVICRM_UF === 'UnitTest' || CRM_Utils_Constant::value('SMARTY_DEBUG_STRINGS')) ? $templateString : ''));
}
finally {
$smarty->caching = $cachingValue;
$smarty->assign('smartySingleUseString');
restore_error_handler();
if ($useSecurityPolicy) {
// $smarty->disableSecurity();
Civi::service('civi.smarty.userContent')->disable();
}
}
return $templateString;
}

Expand Down
2 changes: 1 addition & 1 deletion Civi/Api4/Generic/AbstractAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ protected function evaluateCondition($expr, $vars) {
throw new \API_Exception('Illegal character in expression');
}
$tpl = "{if $expr}1{else}0{/if}";
return (bool) trim(\CRM_Core_Smarty::singleton()->fetchWith('string:' . $tpl, $vars));
return (bool) trim(\CRM_Utils_String::parseOneOffStringThroughSmarty($tpl, $vars));
}

/**
Expand Down

0 comments on commit a88d8bf

Please sign in to comment.