Skip to content

Securing your outputs

HTML/Smarty

Untrusted data placed in HTML must be encoded for HTML output at some point. The PHP function htmlentities() does this, and the Smarty variable modifier escape behaves similarly.

Default smarty escaping

There are 2 strategies for smarty encoding

  1. encode by default, specify strings that should not be escaped
  2. only encode strings that are specified for encoding.

Best practice recommends the former. However, CiviCRM did not implement this from the start so switching to it is challenging. As of CiviCRM 5.46.0 it is possible (but experimental) to enable Smarty escaping by default by adding the following to civicrm.settings.php :

define('CIVICRM_SMARTY_DEFAULT_ESCAPE', TRUE);

What this does is add a default modifier function (CRM_Core_Smarty::escape) that will parse any smarty variables that do not have the smarty:nodefaults modifer (even if they already have escaping applied). The function has a number of early-returns to help us get past common patterns in CiviCRM - these are intended to be transitional as part of a tightening security practice. In addition specific strings that are identified as inappropriate for escaping such as this one should be marked as such in the tpl e.g

{$row.weight|smarty:nodefaults}

There are some gotchas

Gotcha 1 - over-escaping

As this is generally not used in production yet (as of 5.59) the code still has a lot of places where html is escaped but actually needs to render as html. This is a work in progress and PRs like above need to be added

Gotcha 2 - smarty notices

  1. Using isset() in the smarty layer is incompatible with escape-by-default. If the value is not defined it cannot be passed to isset - resulting in a fatal. These have all been removed from CiviCRM core code but not yet message templates and extensions.
  2. Using empty() in the smarty layer will result in e-notices if not defined. Generally the recommendation is to ensure they ARE defined but alternatively the smarty:nodefaults modifier can be passed in.

As can be seen by the above it is better to check all potentially relevant smarty variables are assigned than to check for them in the template.

Ways to deal with smarty Notices are

1) Assign unconditionally e.g instead of 

if ($contactID) {
  $this-assign('contactID', $contactID);
}

use 

$this-assign('contactID', $contactID);

In the real world these might also be array keys of the if logic might be deep and hard to clean-up in which case consider ...

2) Figuring out what values really need to be assigned / what logic the template is actually using/ applying and change the assignments - e.g sometimes something mystical like $useForMember is being checked and after analysing the code what you might need is a php-calculated value to determine if line items should be displayed. $this->assign('isShowLineItems, $this-isShowLineItems())

This approach is good for code cleanup & future us appreciate it but sometimes you just need to...

3) Use array_key_exists in the template layer If you are dealing with a form then in CRM_Core_Form::renderForm() the form is converted to an array and assigned to the template - so anything assigned to the template using $form->assign() (or $this->assign() from with the form) can be accessed in the smarty layer with {if array_key_exists('variableName', $form}$variableName{/if} or for QuickForm fields{if array_key_exists('fieldName', $form}$form.fieldName.html{/if}

You can also use array_key_exists for arrays otherwise assigned.

Note that for a while we were assigning NULL for quick form elements that did not exist - eg using getOptionalQuickFormElements - that is incompatible with array_key_exists

Or if all else fails...

6) Assign with a sledgehammer.

To ensure smartyVariables are assigned you can pass an array of variable names to ensureVariablesAreAssigned. It check if they are already assigned and if not assign them as NULL.

  CRM_Core_Smarty::singleton()->ensureVariablesAreAssigned($this->expectedSmartyVariables);

Between tags

Whether data needs to be output encoded depends primarily on the data source:

Database data between tags

Data which comes directly out of MySQL has already been partially encoded for HTML output. This means that when you place this data between HTML tags, you don't need to perform any output encoding. This means the syntax below is safe - however, be aware that changes to the php code could make the smarty code insecure.

// in page class
$dao = new CRM_Core_DAO();
$dao->query('SELECT display_name FROM civicrm_contact');
if ($dao->fetch()) {
  // set template variable
  $this->assign('displayName', $displayName);
}

<!-- in template -->
<div>{$displayName}</div>

API data or direct user input between tags

HTML output encoding needs to be performed at some point for any data that is fetched via the API as well as any untrusted user input that is placed into HTML before being saved to the database. The recommended approach is to perform output encoding in the template:

// in page class
$contacts = \Civi\Api4\Contact::get()
  ->addSelect('display_name')
  ->execute();
foreach ($contacts as $contact) {
  $this->assign('displayName', $contact['display_name']);
}

<!-- in template -->
<div>{$displayName|escape}</div>

Alternatively, you can encode before passing the value to the template:

// in page class
$contacts = \Civi\Api4\Contact::get()
  ->addSelect('display_name')
  ->execute();
foreach ($contacts as $contact) {
  $this->assign('displayName', htmlentities($contact['display_name']));
}

Tip

Be wary of using user input in error messages. This is a common scenario wherein untrusted user input can end up in HTML with no HTML output encoding.

HTML attributes

When placing data within attributes, always use Smarty's escape variable modifier to encode HTML entities.

<a href="#" title="{$displayName|escape}">Foo</a>

Note

HTML output encoding is always necessary for attribute data (but not always necessary for data between tags) because of the intentionally incomplete input encoding that CiviCRM performs.

Javascript in Smarty

If you have a PHP variable that you'd like to use in Javascript, you can assign it to a Javascript variable in a Smarty template as follows

<div>...</div>
{literal}
<script type="text/javascript">
  var data = {/literal}{$data|@json_encode}{literal};
</script>
{/literal}
<div>...</div>

Notice the use of the @json_encode variable modifier. This provides output encoding for JSON which is important to prevent XSS.

AngularJS templates

The AngularJS Security Guide says:

Do not use user input to generate templates dynamically

This means that if you put an ng-app element in a Smarty template, it's very important that you do not use Smarty to put any user input inside the ng-app element.

For example, the following Smarty template would be a security risk:

<div ng-app="crmCaseType">
  <div ng-view=""></div>
  <div>{$untrustedData}</div>
</div>

This is bad because the $untrustedData PHP variable can contain a string like {{1+2}} which AngularJS will execute, opening the door to XSS vulnerabilities.

SQL

When writing SQL, it is very important to protect against SQL injection by ensuring that all variables are passed into SQL with sufficient validation and encoding. CiviCRM has several functions to help with this process, as described below.

CRM_Core_DAO::executeQuery

$name = 'John Smith'; /* un-trusted data */
$optedOut = 0;        /* un-trusted data */

$query = "
  SELECT id
  FROM civicrm_contact
  WHERE
    display_name like %1 AND
    is_opt_out = %2";

$result = CRM_Core_DAO::executeQuery($query, [
  1 => ['%' . $name . '%', 'String'],
  2 => [$optedOut, 'Integer'],
]);

This example ensures that variables are safely escaped before being inserted into the query. CiviCRM also allows developers to specify the type of variable that should be allowed. In the case of the %2 ($optedOut) parameter, only an Integer input will be permitted.

The variable types available for this can be found in CRM_Utils_Type::validate. The query engine then applies appropriate escaping for the type.

CRM_Utils_Type::escape

In some circumstances you may find that a complex query is easier to build by directly escaping values using the CRM_Utils_Type::escape() method. It is prefereable to use the form above or the CRM_Utils_SQL_Select format

$name = CRM_Utils_Type::escape('John Smith', 'String');
$column = CRM_Utils_Type::escape('civicrm_contact.display_name', 'MysqlColumnNameOrAlias');
$result = CRM_Core_DAO::executeQuery("SELECT FROM civicrm_contact WHERE $column like '%$name%'");

CRM_Utils_SQL_Select

Since CiviCRM 4.7 version there has been an alternate way of generating SQL -- use CRM_Utils_SQL_Select. Compared to plain CRM_Core_DAO, it has three advantages:

  • The syntax uses pithy sigils for escaping strings (@value), numbers (#value) and literal SQL (!value).
  • The escaping for array-data is transparent (e.g. field IN (#listOfNumbers) or field IN (@listOfStrings)).
  • It supports more sophisticated JOIN, GROUP BY, and HAVING clauses.
  • You can build and combine queries in piecemeal fashion with fragment() and merge().
  • The general style of query-building is fluent.

A typical example might look like this:

$dao = CRM_Utils_SQL_Select::from('civicrm_contact c')
  ->join('cm', 'INNER JOIN civicrm_membership cm ON cm.contact_id = c.id')
  ->where('c.contact_type = @ctype', array(
    'ctype' => 'Individual',
  ))
  ->where('cm.membership_type_id IN (#types)', array(
    'types' => array(1, 2, 3, 4),
  ))
  ->where('!column = @value', array(
    'column' => CRM_Utils_Type::escape('cm.status_id', 'MysqlColumnNameOrAlias'),
    'value' => 15,
  ))
  ->execute();

while ($dao->fetch()) { ... }

Equivalently, you may pass all parameters as a separate array:

$dao = CRM_Utils_SQL_Select::from('civicrm_contact c')
  ->join('cm', 'INNER JOIN civicrm_membership cm ON cm.contact_id = c.id')
  ->where('c.contact_type = @ctype')
  ->where('cm.membership_type_id IN (#types)')
  ->where('!column = @value')
  ->param(array(
    'ctype' => 'Individual',
    'types' => array(1, 2, 3, 4),
    'column' => CRM_Utils_Type::escape('cm.status_id', 'MysqlColumnNameOrAlias'),
    'value' => 15,
  ))
  ->execute();

while ($dao->fetch()) { ... }

For convenience, you can chain the execute() with other DAO functions like fetchAll(), fetchValue() or fetchMap().

$records = CRM_Utils_SQL_Select::from('mytable')
  ->select('...')
  ->execute()
  ->fetchAll();

Further information on this method can be found in the CRM_Utils_SQL_Select class

PHP

PHP functions like eval() and many others will convert strings stored in PHP variables into executable PHP code. If untrusted inputs ever make their way into such strings, critical code injection vulnerabilities can arise. It's best to avoid these functions entirely — and fortunately modern PHP developers almost never need to use such functions. In the rare event that you find yourself needing to convert a string to PHP code, you must make certain that untrusted data is strictly validated.

Shell commands

Here are some PHP functions which execute shell commands:

  • exec()
  • passthru()
  • system()
  • shell_exec()
  • popen()
  • proc_open()
  • pcntl_exec()
  • `` (backticks)

Using these functions can be very risky! If you're inclided to use one of these functions, it's best to spend some time looking a way to not use one of the functions. If you really can't find a way around it, then make sure to use escapeshellarg (and in some cases escapeshellcmd) to properly encode data sent to the shell.