diff --git a/docs/security/index.md b/docs/security/index.md index adc4f02ce3b82d8f1fa0b3c375adda05dbf9340c..fd831ef09b01b37d1cf27a2907dab969d4d30c17 100644 --- a/docs/security/index.md +++ b/docs/security/index.md @@ -57,7 +57,7 @@ $displayName = CRM_Core_DAO::executeQuery($query, array( Now, users will only be able to send integers in, and CiviCRM will only be able to send integers out. This is obviously a simplified example, but it illustrates the concepts of inputs, outputs, and sanitizing. -## Sanitization methods +## Sanitization methods {:#sanitization} Sanitizing (also sometimes generally called "**escaping**") refers the process of cleaning (or rejecting) data to protect against attacks. @@ -97,7 +97,7 @@ In rare cases such as user-editable rich text fields, CiviCRM cannot use validat Now that we understand the difference between inputs and outputs, as well as the different sanitization techniques, the question arises: *at what point in my code should I sanitize? Input, or output?* -### In an ideal world +### In an ideal world {:#ideal} Ideally developers should do the following. @@ -112,7 +112,7 @@ Ideally developers should do the following. A common (and well meaning) mistake is to *encode inputs* instead of *encoding outputs*. For example, we might choose to store a string like `"Foo Bar" <foo@example.org>` in the database as `"Foo Bar" <foo@example.org>` because we know that, later on, our application will display it within an HTML page. This approach is bad because different outputs (e.g. HTML, SQL, shell) require different of encoding schemes. During input we have no reliable way of knowing which outputs the data will reach. -### CiviCRM's current strategy +### CiviCRM's current strategy {:#strategy} Unfortunately (at least as of 2017) CiviCRM exists in a somewhat uncomfortable limbo between the ideal world and the misguided world. In some places, CiviCRM sanitizes inputs with a partial encoding for HTML output, and then does not encode the HTML output. In other places, (e.g. in SQL queries) CiviCRM encodes outputs. In 2012, developers [identified the need to improve this situation](https://issues.civicrm.org/jira/browse/CRM-11532), but unfortunately it's not an easy task because shifting strategies has implications across the entire codebase. This doesn't mean CiviCRM is rife with security vulnerabilities — it just means that CiviCRM has not been *consistent* about how it approaches security. @@ -120,20 +120,13 @@ CiviCRM's strategy is as follows: * Inputs: 1. Validate inputs when possible - 1. For rich text: purify inputs with `CRM_Utils_String::purifyHTML`. - 1. For non-rich text: *encode* inputs with `CRM_Utils_API_HTMLInputCoder::encodeInput()` - * Note that this function only encodes `<` and `>`. It does *not* encode quotes. + 1. For non-rich text, [partially encode inputs](/security/inputs.md#input-encoding) + 1. For rich text, [purify inputs](/security/inputs.md#input-purification) * Outputs: 1. HTML: - * Do *not* perform HTML encoding for data between tags - - e.g. `<div>{$displayName}</div>` - - * *Do* perform HTML encoding for data within attributes - - e.g. `<a href="#" title="{$displayName|escape}">Foo</a>` - - 1. SQL: validate and encode - 1. Shell: validate and encode + * Do *not* perform HTML encoding for [data between tags](/security/outputs.md#between-tags) + * *Do* perform HTML encoding for [data within attributes](/security/outputs.md#in-attributes) + 1. SQL: [validate and encode](/security/outputs.md#sql) + 1. Shell: [validate and encode](/security/outputs.md#shell) diff --git a/docs/security/inputs.md b/docs/security/inputs.md index a2bd8eebf5455f3c92c7932b78876f1f220e7026..6bff7a95cbf9d60a974c652a01673142feda7add 100644 --- a/docs/security/inputs.md +++ b/docs/security/inputs.md @@ -1,5 +1,18 @@ # Securing your inputs +## Input encoding {:#input-encoding} + +For almost all inputs, CiviCRM automatically uses `CRM_Utils_API_HTMLInputCoder::encodeInput()` to apply a *partial* encoding for HTML output. This encoding step happens at a low level for inputs passed through the API or the BAO (except for fields noted in `CRM_Utils_API_HTMLInputCoder::getSkipFields()`). So if you're using the API or the BAO to process your input you don't need to do anything special. + +If, for some strange reason, you happen to be writing untrusted data to the database directly with SQL, you should encode this data in a fashion consistent with `CRM_Utils_API_HTMLInputCoder::encodeInput()`. + +Note that `CRM_Utils_API_HTMLInputCoder::encodeInput()` only encodes `<` and `>`. It does *not* encode quotes. This has some special implications for how you should [encode your HTML outputs](/security/outputs.md#html). + +## Input purification {:#input-purification} + +When accepting untrusted data with rich text (uncommon), pass the data through `CRM_Utils_String::purifyHTML` to remove XSS. + + ## `GET` parameters Through the CiviCRM code base you will find that there are a number of times where CiviCRM takes variables passed to it through the URL e.g. `?cid=1234` or `?id=1234`. CiviCRM has put in place some inbuilt functions that help to ensure that no dangerous values are able to be passed through. diff --git a/docs/security/outputs.md b/docs/security/outputs.md index e26f95d4a54d4eccc5bccd1965b4d591090c789a..ae81c1a2ebdb7e223034351160d068b4a6c74262 100644 --- a/docs/security/outputs.md +++ b/docs/security/outputs.md @@ -1,26 +1,34 @@ # Securing your outputs -## In Smarty +## In HTML/Smarty {:#html} -### Between tags +### Between tags {:#between-tags} -TODO +When placing data between tags, no output encoding is necessary. For example: -### In attributes +```html +<div>{$displayName}</div> +``` -TODO +### In attributes {:#in-attributes} + +When placing data within attributes, use Smarty's [escape](https://www.smarty.net/docsv2/en/language.modifier.escape) variable modifier to encode HTML entities. +```html +<a href="#" title="{$displayName|escape}">Foo</a> +``` + +!!! note + HTML output encoding *is* necessary for attribute data (but *not* necessary for data between tags) because of the intentionally incomplete [input encoding](/security/inputs.md#input-encoding) that CiviCRM performs. ## In AngularJS templates TODO -## SQL - -When writing SQL, it is very important that developers protect against [SQL injection](https://en.wikipedia.org/wiki/SQL_injection) by ensuring that all variables are passed into SQL safely and securely. +## SQL {:#sql} -This page describes the inbuilt parameterization tools available for safely executing SQL. +When writing SQL, it is very important to protect against [SQL injection](https://en.wikipedia.org/wiki/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` {:#executeQuery} @@ -122,7 +130,7 @@ TODO https://stackoverflow.com/questions/3115559/exploitable-php-functions -## Shell commands +## Shell commands {:#shell} TODO